[FFmpeg-devel] [PATCH] avcodec: parse options from AVCodec.bsfs

[Carl Eugen Hoyos]

2018-07-19 21:00 GMT+02:00, Aman Gupta <ffmpeg at tmm1.net>:
> On Thu, Jul 19, 2018 at 11:44 AM Carl Eugen Hoyos <ceffmpeg at gmail.com>
> wrote:
>
>> 2018-07-19 3:37 GMT+02:00, James Almer <jamrial at gmail.com>:
>> > On 7/18/2018 3:57 PM, Aman Gupta wrote:
>>
>> >> +            ret = av_opt_set_from_string(s->bsfs[s->nb_bsfs -
>> >> 1]->priv_data, bsf_options_str, shorthand, "=", ":");
>> >> +            if (ret < 0) {
>> >> +                av_log(avctx, AV_LOG_ERROR, "Invalid options for
>> >> bitstream filter %s "
>> >> +                       "requested by the decoder. This is a bug,
>> >> please
>> >> report it.\n",
>> >> +                       bsf_name);
>> >> +                av_freep(&bsf);
>> >> +                ret = AVERROR_BUG;
>> >> +                goto fail;
>> >> +            }
>> >
>> > As i said on IRC, av_opt_set_from_string() can return ENOMEM which is
>> > not a bug in the string contents, so do something like
>> >
>> > if (ret < 0) {
>> >   if (ret != AVERROR(ENOMEM)) {
>> >     av_log(avctx, AV_LOG_ERROR, "Invalid options for bitstream filter %s
>> "
>> >            "requested by the decoder. This is a bug, please report
>> it.\n",
>> >            bsf_name);
>> >     ret = AVERROR_BUG;
>>
>> av_assert(ret == AVERROR(ENOMEM)); ?
>>
>
> I think it's preferable to show the custom error message, since it will
> include the name of the filter and also the name of the codec (via avctx),
> and suggest to the user that they report the bug with both of those details.

Apart from the fact that I fear the message will not trigger the
necessary report (as opposed to the assert): Iirc, you can
convince av_assert to print all this information, no?

Carl Eugen