avconv: fix bitstream filter option parsing

Message ID 20161003194006.6272-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Oct. 3, 2016, 7:40 p.m.
Arguments for bitstream filters weren't being parsed at all, and instead
checked by av_bsf_get_by_name() as if they were part of the filter's name

Signed-off-by: James Almer <jamrial@gmail.com>
---
This should also go into Libav 12, but i'm not CCing libav-stable@libav.org
since i assume it's still for Libav 11 and older for now.

 avconv.c     | 15 ++-------------
 avconv.h     |  1 -
 avconv_opt.c | 45 ++++++++++++++++++++++++++++++++++++---------
 3 files changed, 38 insertions(+), 23 deletions(-)

Comments

James Almer Oct. 10, 2016, 4:34 p.m. | #1
On 10/3/2016 4:40 PM, James Almer wrote:
> Arguments for bitstream filters weren't being parsed at all, and instead
> checked by av_bsf_get_by_name() as if they were part of the filter's name
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> This should also go into Libav 12, but i'm not CCing libav-stable@libav.org
> since i assume it's still for Libav 11 and older for now.
> 
>  avconv.c     | 15 ++-------------
>  avconv.h     |  1 -
>  avconv_opt.c | 45 ++++++++++++++++++++++++++++++++++++---------
>  3 files changed, 38 insertions(+), 23 deletions(-)

Ping.
Anton Khirnov Oct. 10, 2016, 5:23 p.m. | #2
Quoting James Almer (2016-10-03 21:40:06)
> Arguments for bitstream filters weren't being parsed at all, and instead
> checked by av_bsf_get_by_name() as if they were part of the filter's name
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> This should also go into Libav 12, but i'm not CCing libav-stable@libav.org
> since i assume it's still for Libav 11 and older for now.

This is not exactly a "fix", but a new feature -- parameters for
bitstream filters were never supported before. So it probably should not
go into the stable versions.

> 
>  avconv.c     | 15 ++-------------
>  avconv.h     |  1 -
>  avconv_opt.c | 45 ++++++++++++++++++++++++++++++++++++---------
>  3 files changed, 38 insertions(+), 23 deletions(-)
> 
> diff --git a/avconv.c b/avconv.c
> index 0b75cbe..4bd28e6 100644
> --- a/avconv.c
> +++ b/avconv.c
> @@ -190,7 +190,6 @@ static void avconv_cleanup(int ret)
>          for (j = 0; j < ost->nb_bitstream_filters; j++)
>              av_bsf_free(&ost->bsf_ctx[j]);
>          av_freep(&ost->bsf_ctx);
> -        av_freep(&ost->bitstream_filters);
>  
>          av_frame_free(&ost->filtered_frame);
>  
> @@ -1798,17 +1797,8 @@ static int init_output_bsfs(OutputStream *ost)
>      if (!ost->nb_bitstream_filters)
>          return 0;
>  
> -    ost->bsf_ctx = av_mallocz_array(ost->nb_bitstream_filters, sizeof(*ost->bsf_ctx));
> -    if (!ost->bsf_ctx)
> -        return AVERROR(ENOMEM);
> -
>      for (i = 0; i < ost->nb_bitstream_filters; i++) {
> -        ret = av_bsf_alloc(ost->bitstream_filters[i], &ctx);
> -        if (ret < 0) {
> -            av_log(NULL, AV_LOG_ERROR, "Error allocating a bitstream filter context\n");
> -            return ret;
> -        }
> -        ost->bsf_ctx[i] = ctx;
> +        ctx = ost->bsf_ctx[i];
>  
>          ret = avcodec_parameters_copy(ctx->par_in,
>                                        i ? ost->bsf_ctx[i - 1]->par_out : ost->st->codecpar);
> @@ -1820,12 +1810,11 @@ static int init_output_bsfs(OutputStream *ost)
>          ret = av_bsf_init(ctx);
>          if (ret < 0) {
>              av_log(NULL, AV_LOG_ERROR, "Error initializing bitstream filter: %s\n",
> -                   ost->bitstream_filters[i]->name);
> +                   ctx->filter->name);
>              return ret;
>          }
>      }
>  
> -    ctx = ost->bsf_ctx[ost->nb_bitstream_filters - 1];
>      ret = avcodec_parameters_copy(ost->st->codecpar, ctx->par_out);
>      if (ret < 0)
>          return ret;
> diff --git a/avconv.h b/avconv.h
> index fcdf3d0..6360f76 100644
> --- a/avconv.h
> +++ b/avconv.h
> @@ -360,7 +360,6 @@ typedef struct OutputStream {
>      AVRational mux_timebase;
>  
>      int                    nb_bitstream_filters;
> -    const AVBitStreamFilter **bitstream_filters;
>      AVBSFContext            **bsf_ctx;
>  
>      AVCodecContext *enc_ctx;
> diff --git a/avconv_opt.c b/avconv_opt.c
> index 362a5b7..931c35d 100644
> --- a/avconv_opt.c
> +++ b/avconv_opt.c
> @@ -1025,26 +1025,53 @@ static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
>      MATCH_PER_STREAM_OPT(bitstream_filters, str, bsfs, oc, st);
>      while (bsfs && *bsfs) {
>          const AVBitStreamFilter *filter;
> -        char *bsf;
> +        const char *bsf, *bsf_options_str, *bsf_name;
> +        AVDictionary *bsf_options = NULL;
>  
> -        bsf = av_get_token(&bsfs, ",");
> +        bsf = bsf_options_str = av_get_token(&bsfs, ",");
>          if (!bsf)
>              exit_program(1);
> +        bsf_name = av_get_token(&bsf_options_str, "=");
> +        if (!bsf_name)
> +            exit_program(1);
>  
> -        filter = av_bsf_get_by_name(bsf);
> +        filter = av_bsf_get_by_name(bsf_name);
>          if (!filter) {
> -            av_log(NULL, AV_LOG_FATAL, "Unknown bitstream filter %s\n", bsf);
> +            av_log(NULL, AV_LOG_FATAL, "Unknown bitstream filter %s\n", bsf_name);
>              exit_program(1);
>          }
> +        if (*++bsf_options_str) {
> +            ret = av_dict_parse_string(&bsf_options, bsf_options_str, "=", ":", 0);
> +            if (ret < 0) {
> +                av_log(NULL, AV_LOG_ERROR, "Error parsing options for bitstream filter %s\n", bsf_name);
> +                exit_program(1);
> +            }
> +        }
>          av_freep(&bsf);
>  
> -        ost->bitstream_filters = av_realloc_array(ost->bitstream_filters,
> -                                                  ost->nb_bitstream_filters + 1,
> -                                                  sizeof(*ost->bitstream_filters));
> -        if (!ost->bitstream_filters)
> +        ost->bsf_ctx = av_realloc_array(ost->bsf_ctx,
> +                                        ost->nb_bitstream_filters + 1,
> +                                        sizeof(*ost->bsf_ctx));
> +        if (!ost->bsf_ctx)
> +            exit_program(1);
> +
> +        ret = av_bsf_alloc(filter, &ost->bsf_ctx[ost->nb_bitstream_filters]);
> +        if (ret < 0) {
> +            av_log(NULL, AV_LOG_ERROR, "Error allocating a bistream filter context\n");
>              exit_program(1);
> +        }
> +        ost->nb_bitstream_filters++;
> +
> +        if (bsf_options && filter->priv_class) {
> +            ret = av_opt_set_dict(ost->bsf_ctx[ost->nb_bitstream_filters-1]->priv_data, &bsf_options);
> +            if (ret < 0) {
> +                av_log(NULL, AV_LOG_ERROR, "Error setting options for bitstream filter %s\n", bsf_name);
> +                exit_program(1);
> +            }

It should complain loudly if
- options were provided but the filter does not accept any
- some options were not consumed
otherwise we'll again get the old problem of users cargo culting random
options from the internet with no indication that they don't do
anything.

Also, it might be better to use av_opt_set_dict() directly on the filter
context itself, so that this will also work on options that are shared
among all filters (if we add any later).
James Almer Oct. 13, 2016, 2:05 p.m. | #3
On 10/10/2016 2:23 PM, Anton Khirnov wrote:
> Quoting James Almer (2016-10-03 21:40:06)
>> Arguments for bitstream filters weren't being parsed at all, and instead
>> checked by av_bsf_get_by_name() as if they were part of the filter's name
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> This should also go into Libav 12, but i'm not CCing libav-stable@libav.org
>> since i assume it's still for Libav 11 and older for now.
> 
> This is not exactly a "fix", but a new feature -- parameters for
> bitstream filters were never supported before. So it probably should not
> go into the stable versions.
> 
>>
>>  avconv.c     | 15 ++-------------
>>  avconv.h     |  1 -
>>  avconv_opt.c | 45 ++++++++++++++++++++++++++++++++++++---------
>>  3 files changed, 38 insertions(+), 23 deletions(-)
>>
>> diff --git a/avconv.c b/avconv.c
>> index 0b75cbe..4bd28e6 100644
>> --- a/avconv.c
>> +++ b/avconv.c
>> @@ -190,7 +190,6 @@ static void avconv_cleanup(int ret)
>>          for (j = 0; j < ost->nb_bitstream_filters; j++)
>>              av_bsf_free(&ost->bsf_ctx[j]);
>>          av_freep(&ost->bsf_ctx);
>> -        av_freep(&ost->bitstream_filters);
>>  
>>          av_frame_free(&ost->filtered_frame);
>>  
>> @@ -1798,17 +1797,8 @@ static int init_output_bsfs(OutputStream *ost)
>>      if (!ost->nb_bitstream_filters)
>>          return 0;
>>  
>> -    ost->bsf_ctx = av_mallocz_array(ost->nb_bitstream_filters, sizeof(*ost->bsf_ctx));
>> -    if (!ost->bsf_ctx)
>> -        return AVERROR(ENOMEM);
>> -
>>      for (i = 0; i < ost->nb_bitstream_filters; i++) {
>> -        ret = av_bsf_alloc(ost->bitstream_filters[i], &ctx);
>> -        if (ret < 0) {
>> -            av_log(NULL, AV_LOG_ERROR, "Error allocating a bitstream filter context\n");
>> -            return ret;
>> -        }
>> -        ost->bsf_ctx[i] = ctx;
>> +        ctx = ost->bsf_ctx[i];
>>  
>>          ret = avcodec_parameters_copy(ctx->par_in,
>>                                        i ? ost->bsf_ctx[i - 1]->par_out : ost->st->codecpar);
>> @@ -1820,12 +1810,11 @@ static int init_output_bsfs(OutputStream *ost)
>>          ret = av_bsf_init(ctx);
>>          if (ret < 0) {
>>              av_log(NULL, AV_LOG_ERROR, "Error initializing bitstream filter: %s\n",
>> -                   ost->bitstream_filters[i]->name);
>> +                   ctx->filter->name);
>>              return ret;
>>          }
>>      }
>>  
>> -    ctx = ost->bsf_ctx[ost->nb_bitstream_filters - 1];
>>      ret = avcodec_parameters_copy(ost->st->codecpar, ctx->par_out);
>>      if (ret < 0)
>>          return ret;
>> diff --git a/avconv.h b/avconv.h
>> index fcdf3d0..6360f76 100644
>> --- a/avconv.h
>> +++ b/avconv.h
>> @@ -360,7 +360,6 @@ typedef struct OutputStream {
>>      AVRational mux_timebase;
>>  
>>      int                    nb_bitstream_filters;
>> -    const AVBitStreamFilter **bitstream_filters;
>>      AVBSFContext            **bsf_ctx;
>>  
>>      AVCodecContext *enc_ctx;
>> diff --git a/avconv_opt.c b/avconv_opt.c
>> index 362a5b7..931c35d 100644
>> --- a/avconv_opt.c
>> +++ b/avconv_opt.c
>> @@ -1025,26 +1025,53 @@ static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
>>      MATCH_PER_STREAM_OPT(bitstream_filters, str, bsfs, oc, st);
>>      while (bsfs && *bsfs) {
>>          const AVBitStreamFilter *filter;
>> -        char *bsf;
>> +        const char *bsf, *bsf_options_str, *bsf_name;
>> +        AVDictionary *bsf_options = NULL;
>>  
>> -        bsf = av_get_token(&bsfs, ",");
>> +        bsf = bsf_options_str = av_get_token(&bsfs, ",");
>>          if (!bsf)
>>              exit_program(1);
>> +        bsf_name = av_get_token(&bsf_options_str, "=");
>> +        if (!bsf_name)
>> +            exit_program(1);
>>  
>> -        filter = av_bsf_get_by_name(bsf);
>> +        filter = av_bsf_get_by_name(bsf_name);
>>          if (!filter) {
>> -            av_log(NULL, AV_LOG_FATAL, "Unknown bitstream filter %s\n", bsf);
>> +            av_log(NULL, AV_LOG_FATAL, "Unknown bitstream filter %s\n", bsf_name);
>>              exit_program(1);
>>          }
>> +        if (*++bsf_options_str) {
>> +            ret = av_dict_parse_string(&bsf_options, bsf_options_str, "=", ":", 0);
>> +            if (ret < 0) {
>> +                av_log(NULL, AV_LOG_ERROR, "Error parsing options for bitstream filter %s\n", bsf_name);
>> +                exit_program(1);
>> +            }
>> +        }
>>          av_freep(&bsf);
>>  
>> -        ost->bitstream_filters = av_realloc_array(ost->bitstream_filters,
>> -                                                  ost->nb_bitstream_filters + 1,
>> -                                                  sizeof(*ost->bitstream_filters));
>> -        if (!ost->bitstream_filters)
>> +        ost->bsf_ctx = av_realloc_array(ost->bsf_ctx,
>> +                                        ost->nb_bitstream_filters + 1,
>> +                                        sizeof(*ost->bsf_ctx));
>> +        if (!ost->bsf_ctx)
>> +            exit_program(1);
>> +
>> +        ret = av_bsf_alloc(filter, &ost->bsf_ctx[ost->nb_bitstream_filters]);
>> +        if (ret < 0) {
>> +            av_log(NULL, AV_LOG_ERROR, "Error allocating a bistream filter context\n");
>>              exit_program(1);
>> +        }
>> +        ost->nb_bitstream_filters++;
>> +
>> +        if (bsf_options && filter->priv_class) {
>> +            ret = av_opt_set_dict(ost->bsf_ctx[ost->nb_bitstream_filters-1]->priv_data, &bsf_options);
>> +            if (ret < 0) {
>> +                av_log(NULL, AV_LOG_ERROR, "Error setting options for bitstream filter %s\n", bsf_name);
>> +                exit_program(1);
>> +            }
> 
> It should complain loudly if
> - options were provided but the filter does not accept any

Sure. Should be a matter of checking if filter->priv_class is not NULL.

> - some options were not consumed

Can you tell me how to achieve this? I don't use AVDictionary often.

> otherwise we'll again get the old problem of users cargo culting random
> options from the internet with no indication that they don't do
> anything.
> 
> Also, it might be better to use av_opt_set_dict() directly on the filter
> context itself, so that this will also work on options that are shared
> among all filters (if we add any later).
>
Anton Khirnov Oct. 13, 2016, 2:26 p.m. | #4
Quoting James Almer (2016-10-13 16:05:31)
> On 10/10/2016 2:23 PM, Anton Khirnov wrote:
> > Quoting James Almer (2016-10-03 21:40:06)
> >> Arguments for bitstream filters weren't being parsed at all, and instead
> >> checked by av_bsf_get_by_name() as if they were part of the filter's name
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >> This should also go into Libav 12, but i'm not CCing libav-stable@libav.org
> >> since i assume it's still for Libav 11 and older for now.
> > 
> > This is not exactly a "fix", but a new feature -- parameters for
> > bitstream filters were never supported before. So it probably should not
> > go into the stable versions.
> > 
> >>
> >>  avconv.c     | 15 ++-------------
> >>  avconv.h     |  1 -
> >>  avconv_opt.c | 45 ++++++++++++++++++++++++++++++++++++---------
> >>  3 files changed, 38 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/avconv.c b/avconv.c
> >> index 0b75cbe..4bd28e6 100644
> >> --- a/avconv.c
> >> +++ b/avconv.c
> >> @@ -190,7 +190,6 @@ static void avconv_cleanup(int ret)
> >>          for (j = 0; j < ost->nb_bitstream_filters; j++)
> >>              av_bsf_free(&ost->bsf_ctx[j]);
> >>          av_freep(&ost->bsf_ctx);
> >> -        av_freep(&ost->bitstream_filters);
> >>  
> >>          av_frame_free(&ost->filtered_frame);
> >>  
> >> @@ -1798,17 +1797,8 @@ static int init_output_bsfs(OutputStream *ost)
> >>      if (!ost->nb_bitstream_filters)
> >>          return 0;
> >>  
> >> -    ost->bsf_ctx = av_mallocz_array(ost->nb_bitstream_filters, sizeof(*ost->bsf_ctx));
> >> -    if (!ost->bsf_ctx)
> >> -        return AVERROR(ENOMEM);
> >> -
> >>      for (i = 0; i < ost->nb_bitstream_filters; i++) {
> >> -        ret = av_bsf_alloc(ost->bitstream_filters[i], &ctx);
> >> -        if (ret < 0) {
> >> -            av_log(NULL, AV_LOG_ERROR, "Error allocating a bitstream filter context\n");
> >> -            return ret;
> >> -        }
> >> -        ost->bsf_ctx[i] = ctx;
> >> +        ctx = ost->bsf_ctx[i];
> >>  
> >>          ret = avcodec_parameters_copy(ctx->par_in,
> >>                                        i ? ost->bsf_ctx[i - 1]->par_out : ost->st->codecpar);
> >> @@ -1820,12 +1810,11 @@ static int init_output_bsfs(OutputStream *ost)
> >>          ret = av_bsf_init(ctx);
> >>          if (ret < 0) {
> >>              av_log(NULL, AV_LOG_ERROR, "Error initializing bitstream filter: %s\n",
> >> -                   ost->bitstream_filters[i]->name);
> >> +                   ctx->filter->name);
> >>              return ret;
> >>          }
> >>      }
> >>  
> >> -    ctx = ost->bsf_ctx[ost->nb_bitstream_filters - 1];
> >>      ret = avcodec_parameters_copy(ost->st->codecpar, ctx->par_out);
> >>      if (ret < 0)
> >>          return ret;
> >> diff --git a/avconv.h b/avconv.h
> >> index fcdf3d0..6360f76 100644
> >> --- a/avconv.h
> >> +++ b/avconv.h
> >> @@ -360,7 +360,6 @@ typedef struct OutputStream {
> >>      AVRational mux_timebase;
> >>  
> >>      int                    nb_bitstream_filters;
> >> -    const AVBitStreamFilter **bitstream_filters;
> >>      AVBSFContext            **bsf_ctx;
> >>  
> >>      AVCodecContext *enc_ctx;
> >> diff --git a/avconv_opt.c b/avconv_opt.c
> >> index 362a5b7..931c35d 100644
> >> --- a/avconv_opt.c
> >> +++ b/avconv_opt.c
> >> @@ -1025,26 +1025,53 @@ static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
> >>      MATCH_PER_STREAM_OPT(bitstream_filters, str, bsfs, oc, st);
> >>      while (bsfs && *bsfs) {
> >>          const AVBitStreamFilter *filter;
> >> -        char *bsf;
> >> +        const char *bsf, *bsf_options_str, *bsf_name;
> >> +        AVDictionary *bsf_options = NULL;
> >>  
> >> -        bsf = av_get_token(&bsfs, ",");
> >> +        bsf = bsf_options_str = av_get_token(&bsfs, ",");
> >>          if (!bsf)
> >>              exit_program(1);
> >> +        bsf_name = av_get_token(&bsf_options_str, "=");
> >> +        if (!bsf_name)
> >> +            exit_program(1);
> >>  
> >> -        filter = av_bsf_get_by_name(bsf);
> >> +        filter = av_bsf_get_by_name(bsf_name);
> >>          if (!filter) {
> >> -            av_log(NULL, AV_LOG_FATAL, "Unknown bitstream filter %s\n", bsf);
> >> +            av_log(NULL, AV_LOG_FATAL, "Unknown bitstream filter %s\n", bsf_name);
> >>              exit_program(1);
> >>          }
> >> +        if (*++bsf_options_str) {
> >> +            ret = av_dict_parse_string(&bsf_options, bsf_options_str, "=", ":", 0);
> >> +            if (ret < 0) {
> >> +                av_log(NULL, AV_LOG_ERROR, "Error parsing options for bitstream filter %s\n", bsf_name);
> >> +                exit_program(1);
> >> +            }
> >> +        }
> >>          av_freep(&bsf);
> >>  
> >> -        ost->bitstream_filters = av_realloc_array(ost->bitstream_filters,
> >> -                                                  ost->nb_bitstream_filters + 1,
> >> -                                                  sizeof(*ost->bitstream_filters));
> >> -        if (!ost->bitstream_filters)
> >> +        ost->bsf_ctx = av_realloc_array(ost->bsf_ctx,
> >> +                                        ost->nb_bitstream_filters + 1,
> >> +                                        sizeof(*ost->bsf_ctx));
> >> +        if (!ost->bsf_ctx)
> >> +            exit_program(1);
> >> +
> >> +        ret = av_bsf_alloc(filter, &ost->bsf_ctx[ost->nb_bitstream_filters]);
> >> +        if (ret < 0) {
> >> +            av_log(NULL, AV_LOG_ERROR, "Error allocating a bistream filter context\n");
> >>              exit_program(1);
> >> +        }
> >> +        ost->nb_bitstream_filters++;
> >> +
> >> +        if (bsf_options && filter->priv_class) {
> >> +            ret = av_opt_set_dict(ost->bsf_ctx[ost->nb_bitstream_filters-1]->priv_data, &bsf_options);
> >> +            if (ret < 0) {
> >> +                av_log(NULL, AV_LOG_ERROR, "Error setting options for bitstream filter %s\n", bsf_name);
> >> +                exit_program(1);
> >> +            }
> > 
> > It should complain loudly if
> > - options were provided but the filter does not accept any
> 
> Sure. Should be a matter of checking if filter->priv_class is not NULL.
> 
> > - some options were not consumed
> 
> Can you tell me how to achieve this? I don't use AVDictionary often.

assert_avoptions() does exactly this. And if you implement my last
suggestion, you won't even need to check for priv_class, just the call
to assert_avoptions() will handle both cases.

Patch

diff --git a/avconv.c b/avconv.c
index 0b75cbe..4bd28e6 100644
--- a/avconv.c
+++ b/avconv.c
@@ -190,7 +190,6 @@  static void avconv_cleanup(int ret)
         for (j = 0; j < ost->nb_bitstream_filters; j++)
             av_bsf_free(&ost->bsf_ctx[j]);
         av_freep(&ost->bsf_ctx);
-        av_freep(&ost->bitstream_filters);
 
         av_frame_free(&ost->filtered_frame);
 
@@ -1798,17 +1797,8 @@  static int init_output_bsfs(OutputStream *ost)
     if (!ost->nb_bitstream_filters)
         return 0;
 
-    ost->bsf_ctx = av_mallocz_array(ost->nb_bitstream_filters, sizeof(*ost->bsf_ctx));
-    if (!ost->bsf_ctx)
-        return AVERROR(ENOMEM);
-
     for (i = 0; i < ost->nb_bitstream_filters; i++) {
-        ret = av_bsf_alloc(ost->bitstream_filters[i], &ctx);
-        if (ret < 0) {
-            av_log(NULL, AV_LOG_ERROR, "Error allocating a bitstream filter context\n");
-            return ret;
-        }
-        ost->bsf_ctx[i] = ctx;
+        ctx = ost->bsf_ctx[i];
 
         ret = avcodec_parameters_copy(ctx->par_in,
                                       i ? ost->bsf_ctx[i - 1]->par_out : ost->st->codecpar);
@@ -1820,12 +1810,11 @@  static int init_output_bsfs(OutputStream *ost)
         ret = av_bsf_init(ctx);
         if (ret < 0) {
             av_log(NULL, AV_LOG_ERROR, "Error initializing bitstream filter: %s\n",
-                   ost->bitstream_filters[i]->name);
+                   ctx->filter->name);
             return ret;
         }
     }
 
-    ctx = ost->bsf_ctx[ost->nb_bitstream_filters - 1];
     ret = avcodec_parameters_copy(ost->st->codecpar, ctx->par_out);
     if (ret < 0)
         return ret;
diff --git a/avconv.h b/avconv.h
index fcdf3d0..6360f76 100644
--- a/avconv.h
+++ b/avconv.h
@@ -360,7 +360,6 @@  typedef struct OutputStream {
     AVRational mux_timebase;
 
     int                    nb_bitstream_filters;
-    const AVBitStreamFilter **bitstream_filters;
     AVBSFContext            **bsf_ctx;
 
     AVCodecContext *enc_ctx;
diff --git a/avconv_opt.c b/avconv_opt.c
index 362a5b7..931c35d 100644
--- a/avconv_opt.c
+++ b/avconv_opt.c
@@ -1025,26 +1025,53 @@  static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
     MATCH_PER_STREAM_OPT(bitstream_filters, str, bsfs, oc, st);
     while (bsfs && *bsfs) {
         const AVBitStreamFilter *filter;
-        char *bsf;
+        const char *bsf, *bsf_options_str, *bsf_name;
+        AVDictionary *bsf_options = NULL;
 
-        bsf = av_get_token(&bsfs, ",");
+        bsf = bsf_options_str = av_get_token(&bsfs, ",");
         if (!bsf)
             exit_program(1);
+        bsf_name = av_get_token(&bsf_options_str, "=");
+        if (!bsf_name)
+            exit_program(1);
 
-        filter = av_bsf_get_by_name(bsf);
+        filter = av_bsf_get_by_name(bsf_name);
         if (!filter) {
-            av_log(NULL, AV_LOG_FATAL, "Unknown bitstream filter %s\n", bsf);
+            av_log(NULL, AV_LOG_FATAL, "Unknown bitstream filter %s\n", bsf_name);
             exit_program(1);
         }
+        if (*++bsf_options_str) {
+            ret = av_dict_parse_string(&bsf_options, bsf_options_str, "=", ":", 0);
+            if (ret < 0) {
+                av_log(NULL, AV_LOG_ERROR, "Error parsing options for bitstream filter %s\n", bsf_name);
+                exit_program(1);
+            }
+        }
         av_freep(&bsf);
 
-        ost->bitstream_filters = av_realloc_array(ost->bitstream_filters,
-                                                  ost->nb_bitstream_filters + 1,
-                                                  sizeof(*ost->bitstream_filters));
-        if (!ost->bitstream_filters)
+        ost->bsf_ctx = av_realloc_array(ost->bsf_ctx,
+                                        ost->nb_bitstream_filters + 1,
+                                        sizeof(*ost->bsf_ctx));
+        if (!ost->bsf_ctx)
+            exit_program(1);
+
+        ret = av_bsf_alloc(filter, &ost->bsf_ctx[ost->nb_bitstream_filters]);
+        if (ret < 0) {
+            av_log(NULL, AV_LOG_ERROR, "Error allocating a bistream filter context\n");
             exit_program(1);
+        }
+        ost->nb_bitstream_filters++;
+
+        if (bsf_options && filter->priv_class) {
+            ret = av_opt_set_dict(ost->bsf_ctx[ost->nb_bitstream_filters-1]->priv_data, &bsf_options);
+            if (ret < 0) {
+                av_log(NULL, AV_LOG_ERROR, "Error setting options for bitstream filter %s\n", bsf_name);
+                exit_program(1);
+            }
+        }
+        av_dict_free(&bsf_options);
+        av_freep(&bsf_name);
 
-        ost->bitstream_filters[ost->nb_bitstream_filters++] = filter;
         if (*bsfs)
             bsfs++;
     }