On Tue, Aug 09, 2016 at 12:51:30AM +0200, Carl Eugen Hoyos wrote:
> Hi!
>
> 2016-08-05 12:44 GMT+02:00 Nicolas George <geo...@nsup.org>:
> > Le nonidi 19 thermidor, an CCXXIV, Carl Eugen Hoyos a écrit :
> >> Hi!
> >>
> >> Attached patch implements RFC 2586.
> >>
> >> Please comment, Carl Eugen
> >
> >> From ba470c643c836826d75854e3e3539eb09ddd288a Mon Sep 17 00:00:00 2001
> >> From: Carl Eugen Hoyos <ceho...@ag.or.at>
> >> Date: Fri, 5 Aug 2016 12:22:17 +0200
> >> Subject: [PATCH] lavf/pcmdec: Map mime_type audio/L16 to s16le as specified
> >> in RFC 2586.
> >>
> >> ---
> >> libavformat/pcmdec.c | 63
> >> +++++++++++++++++++++++++++++++++-----------------
> >> 1 file changed, 42 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/libavformat/pcmdec.c b/libavformat/pcmdec.c
> >> index df94345..36ef2c2 100644
> >> --- a/libavformat/pcmdec.c
> >> +++ b/libavformat/pcmdec.c
> >> @@ -36,6 +36,7 @@ static int pcm_read_header(AVFormatContext *s)
> >> {
> >> PCMAudioDemuxerContext *s1 = s->priv_data;
> >> AVStream *st;
> >> + uint8_t *mime_type_opt = NULL;
> >>
> >> st = avformat_new_stream(s, NULL);
> >> if (!st)
> >> @@ -47,6 +48,25 @@ static int pcm_read_header(AVFormatContext *s)
> >> st->codecpar->sample_rate = s1->sample_rate;
> >> st->codecpar->channels = s1->channels;
> >>
> >> + av_opt_get(s->pb, "mime_type", AV_OPT_SEARCH_CHILDREN,
> >> &mime_type_opt);
> >> + if (mime_type_opt) {
> >
> >> + const char *mime_type = mime_type_opt;
> >
> > I do not understand the need for that line.
>
> Removed.
>
> >> + size_t len = strlen(s->iformat->mime_type);
> >
> > Maybe I am missing something obvious, but I think s->iformat->mime_type is
> > NULL for all the other formats. It needs to be checked.
>
> Added the check.
>
> >> + int rate, channels = 0;
> >> + if (!av_strncasecmp(s->iformat->mime_type, mime_type, len)) {
> >
> >> + if ( !sscanf(mime_type + len, ";rate=%d;channels=%d",
> >> &rate, &channels)
> >
> > If I understand the way MIME type works, ";channels=2;rate=48000" would be
> > exactly as valid, and spaces can surround the semicolons.
>
> I changed this hunk.
>
> >> + || !rate) {
> >> + av_log(s, AV_LOG_ERROR,
> >> + "Invalid sample_rate found in mime_type \"%s\"\n",
> >> + mime_type);
> >> + return AVERROR_INVALIDDATA;
> >> + }
> >> + st->codecpar->sample_rate = rate;
> >> + if (channels)
> >> + st->codecpar->channels = channels;
> >> + }
> >> + }
> >> +
> >> st->codecpar->bits_per_coded_sample =
> >> av_get_bits_per_sample(st->codecpar->codec_id);
> >>
> >> @@ -65,7 +85,7 @@ static const AVOption pcm_options[] = {
> >> { NULL },
> >> };
> >>
> >> -#define PCMDEF(name_, long_name_, ext, codec) \
> >
> >> +#define PCMDEF(name_, long_name_, ext, codec, mime_type_) \
> >
> > Instead of changing PCMDEF and all the subsequent declarations, you can
> > create a new macro PCMDEF_WITH_MIME.
> >
> > Even simpler: make PCMDEF varadic, add __ARGS__ in the structure definition.
> > Then, adding the MIME type is just a matter of adding ".mime_type = ..." in
> > the macro call.
>
> Yes, much simpler.
>
> New patch attached.
>
> Thank you, Carl Eugen
> pcmdec.c | 34 ++++++++++++++++++++++++++++++++--
> version.h | 2 +-
> 2 files changed, 33 insertions(+), 3 deletions(-)
> 069eb6ba5db3434e3cc6abca155d8a712c54a8e3
> 0002-lavf-pcmdec-Map-mime_type-audio-L16-to-the-s16le-dem.patch
> From 3ef0951db492f1425a574aac71fe73f144c2d99a Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceho...@ag.or.at>
> Date: Tue, 9 Aug 2016 00:46:57 +0200
> Subject: [PATCH 2/2] lavf/pcmdec: Map mime_type audio/L16 to the s16le
> demuxer as specified in RFC 2586.
>
> ---
> libavformat/pcmdec.c | 34 ++++++++++++++++++++++++++++++++--
> libavformat/version.h | 2 +-
> 2 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/pcmdec.c b/libavformat/pcmdec.c
> index df94345..e3cc2ae 100644
> --- a/libavformat/pcmdec.c
> +++ b/libavformat/pcmdec.c
> @@ -36,6 +36,7 @@ static int pcm_read_header(AVFormatContext *s)
> {
> PCMAudioDemuxerContext *s1 = s->priv_data;
> AVStream *st;
> + uint8_t *mime_type = NULL;
>
> st = avformat_new_stream(s, NULL);
> if (!st)
> @@ -47,6 +48,34 @@ static int pcm_read_header(AVFormatContext *s)
> st->codecpar->sample_rate = s1->sample_rate;
> st->codecpar->channels = s1->channels;
>
> + av_opt_get(s->pb, "mime_type", AV_OPT_SEARCH_CHILDREN, &mime_type);
> + if (mime_type && s->iformat->mime_type) {
> + int rate = 0, channels = 0;
> + size_t len = strlen(s->iformat->mime_type);
> + if (!strncmp(s->iformat->mime_type, mime_type, len)) {
> + uint8_t *options = mime_type + len;
> + len = strlen(mime_type);
> + while (options < mime_type + len) {
> + options = strstr(options, ";");
> + if (!options++)
> + break;
> + if (!rate)
> + sscanf(options, " rate=%d", &rate);
> + if (!channels)
> + sscanf(options, " channels=%d", &channels);
> + }
rate and channels probably should be subject to some sanity checks
like < 0
(unless its checked later)
no further comments from me
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
