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
