[FFmpeg-devel] [PATCH] avcodec: parse options from AVCodec.bsfs

[Aman Gupta]

On Wed, Jul 18, 2018 at 6:38 PM James Almer <jamrial at gmail.com> wrote:

> On 7/18/2018 3:57 PM, Aman Gupta wrote:
> > From: Aman Gupta <aman at tmm1.net>
> >
>
> avcodec/decode: parse options from AVCodec.bsfs
>
> > Fixes a bug that would prevent using multiple comma-separated filters,
> > and allows options to be passed to each filter.
> >
> > Based on similar loop in ffmpeg_opt.c's new_output_stream().
> >
> > Signed-off-by: Aman Gupta <aman at tmm1.net>
> > ---
> >  libavcodec/decode.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 40 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > index 6a3a4df179..67b7443b9d 100644
> > --- a/libavcodec/decode.c
> > +++ b/libavcodec/decode.c
> > @@ -36,6 +36,7 @@
> >  #include "libavutil/imgutils.h"
> >  #include "libavutil/internal.h"
> >  #include "libavutil/intmath.h"
> > +#include "libavutil/opt.h"
> >
> >  #include "avcodec.h"
> >  #include "bytestream.h"
> > @@ -195,27 +196,33 @@ static int bsfs_init(AVCodecContext *avctx)
> >      while (bsfs_str && *bsfs_str) {
> >          AVBSFContext **tmp;
> >          const AVBitStreamFilter *filter;
> > -        char *bsf;
> > +        char *bsf, *bsf_options_str, *bsf_name;
> >
> >          bsf = av_get_token(&bsfs_str, ",");
> >          if (!bsf) {
> >              ret = AVERROR(ENOMEM);
> >              goto fail;
> >          }
> > +        bsf_name = av_strtok(bsf, "=", &bsf_options_str);
> > +        if (!bsf_name) {
> > +            av_freep(&bsf);
> > +            ret = AVERROR(ENOMEM);
> > +            goto fail;
> > +        }
> >
> > -        filter = av_bsf_get_by_name(bsf);
> > +        filter = av_bsf_get_by_name(bsf_name);
> >          if (!filter) {
> >              av_log(avctx, AV_LOG_ERROR, "A non-existing bitstream
> filter %s "
> >                     "requested by a decoder. This is a bug, please
> report it.\n",
> > -                   bsf);
> > -            ret = AVERROR_BUG;
> > +                   bsf_name);
> >              av_freep(&bsf);
> > +            ret = AVERROR_BUG;
> >              goto fail;
> >          }
> > -        av_freep(&bsf);
> >
> >          tmp = av_realloc_array(s->bsfs, s->nb_bsfs + 1,
> sizeof(*s->bsfs));
> >          if (!tmp) {
> > +            av_freep(&bsf);
> >              ret = AVERROR(ENOMEM);
> >              goto fail;
> >          }
> > @@ -223,8 +230,10 @@ static int bsfs_init(AVCodecContext *avctx)
> >          s->nb_bsfs++;
> >
> >          ret = av_bsf_alloc(filter, &s->bsfs[s->nb_bsfs - 1]);
> > -        if (ret < 0)
> > +        if (ret < 0) {
> > +            av_freep(&bsf);
> >              goto fail;
> > +        }
> >
> >          if (s->nb_bsfs == 1) {
> >              /* We do not currently have an API for passing the input
> timebase into decoders,
> > @@ -238,12 +247,36 @@ static int bsfs_init(AVCodecContext *avctx)
> >              ret = avcodec_parameters_copy(s->bsfs[s->nb_bsfs -
> 1]->par_in,
> >                                            s->bsfs[s->nb_bsfs -
> 2]->par_out);
> >          }
> > -        if (ret < 0)
> > +        if (ret < 0) {
> > +            av_freep(&bsf);
> >              goto fail;
> > +        }
> > +
> > +        if (bsf_options_str && filter->priv_class) {
> > +            const AVOption *opt = av_opt_next(s->bsfs[s->nb_bsfs -
> 1]->priv_data, NULL);
> > +            const char * shorthand[2] = {NULL};
> > +
> > +            if (opt)
> > +                shorthand[0] = opt->name;
> > +
> > +            ret = av_opt_set_from_string(s->bsfs[s->nb_bsfs -
> 1]->priv_data, bsf_options_str, shorthand, "=", ":");
> > +            if (ret < 0) {
> > +                av_log(avctx, AV_LOG_ERROR, "Invalid options for
> bitstream filter %s "
> > +                       "requested by the decoder. This is a bug, please
> report it.\n",
> > +                       bsf_name);
> > +                av_freep(&bsf);
> > +                ret = AVERROR_BUG;
> > +                goto fail;
> > +            }
>
> As i said on IRC, av_opt_set_from_string() can return ENOMEM which is
> not a bug in the string contents, so do something like
>
> if (ret < 0) {
>   if (ret != AVERROR(ENOMEM)) {
>     av_log(avctx, AV_LOG_ERROR, "Invalid options for bitstream filter %s "
>            "requested by the decoder. This is a bug, please report it.\n",
>            bsf_name);
>     ret = AVERROR_BUG;
>   }
>   av_freep(&bsf);
>   goto fail;
> }
>

Thanks, fixed and applied.


>
> > +        }
> > +        av_freep(&bsf);
> >
> >          ret = av_bsf_init(s->bsfs[s->nb_bsfs - 1]);
> >          if (ret < 0)
> >              goto fail;
> > +
> > +        if (*bsfs_str)
> > +            bsfs_str++;
> >      }
> >
> >      return 0;
> >
>
> Should be ok otherwise.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>