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. > 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). > _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel