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 <jamr...@gmail.com>
> >> ---
> >> This should also go into Libav 12, but i'm not CCing libav-sta...@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.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
