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