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

["Jan Ekström" <jeebjp@gmail.com>]

On Sun, May 15, 2022 at 9:51 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> 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.
>

Thanks for the stats. Somehow in my mind there was the mantra of "it
is not the start of a sentence, so do not capitalize it", and that
seemed to be the general consensus within the people I've mostly
communicated regarding this subject. The following paragraphs have
full sentences, which is why those would of course be capitalized.

Funny enough checking out
https://www.ffmpeg.org/developer.html#Patches_002fCommitting , the
example actually has a capital letter there in the short explanation
:) .

> > 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.
>

Pretty much, in my mind just capitalizing AV_OPT and having flags as a
normal word just points towards the incorrect name space of the flags,
as opposed to AV_OPT_FLAG_* being a long fully capitalized identifier.
As I said, this is purely a preference thing.

Jan
_______________________________________________
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".