[FFmpeg-devel] [PATCH]lavf/pcmdec: Map mime_type audio/L16 to the s16le demuxer

[Michael Niedermayer]

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 <george at 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 <cehoyos at 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 <cehoyos at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160817/7d444974/attachment.sig>