Re: [FFmpeg-devel] [PATCH] avformat/hls, dashdec: Don't use AV_OPT_FLAG_* in av_dict_set()

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Jan Ekström:
> On Sun, May 15, 2022 at 8:57 PM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
>>
>> av_dict_set() expects a different set of flags, namely the AV_DICT_*
>> flags. Using AV_OPT_FLAG_DECODING_PARAM (or any AV_OPT_FLAG_*) ic
>> av_dict_set() is therefore completely wrong and given that av_dict_set()
>> just doesn't care about whether the string it receives has anything
>> to do with a decoding parameter or not, it should just be removed
>> without replacement.
>> (The numerical value of AV_OPT_FLAG_DECODING_PARAM currently coincides
>> with AV_DICT_IGNORE_SUFFIX. Given that the dictionaries we are dealing
>> with here are always empty (i.e. NULL) before the calls to
>> av_dict_set(), this flag changes nothing. It would be different if
>> it were equal to one of the AV_DICT_DONT_STRDUP_* values.)
> 
> LGTM as far as the patch contents go. Well noticed.
> 
> For commit message I think we utilize do not utilize capital letters
> after the "prefix: ". This can be changed as you apply the patch :) .
> 

"we"? I always use capital letters here; so does Michael Niedermayer
(mostly). In the last two years, 4679 patches used a capital letter
after the ':', whereas 4409 used a non-capital letter. Granted, 3286 of
the 4679 are from me, but I don't think we have a policy here. Neither
should we.

> Personally I would have worded it a la "don't use AV_OPT flags with
> av_dict_set()", but this is just a preference and you are free to
> ignore this bit.

And I thought I wrote it in that way.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".