| Message ID | 20240702090917.319956-3-ffmpeg-devel@pileofstuff.org |
|---|---|
| State | New |
| Headers | show |
| Series | s/RUNTIME/POST_INIT_SETTABLE/ | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | fail | Make fate failed |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Quoting Andrew Sayers (2024-07-02 11:08:39) > An inattentive user might not see the explanation at the top of this file. > Paste the explanation to all the places they might see it. Duplication is bad, and the premise doesn't work anyway. Inattentive users will happily ignore arbitrary amounts of text. In fact the more text there is, the more of it they will ignore. Meanwhile you make actual information harder to find for people who actually bother to read.
On Tue, Jul 02, 2024 at 11:52:29AM +0200, Anton Khirnov wrote: > Quoting Andrew Sayers (2024-07-02 11:08:39) > > An inattentive user might not see the explanation at the top of this file. > > Paste the explanation to all the places they might see it. > > Duplication is bad, and the premise doesn't work anyway. Inattentive > users will happily ignore arbitrary amounts of text. In fact the more > text there is, the more of it they will ignore. > Meanwhile you make actual information harder to find for people who > actually bother to read. That's a reasonable argument, but incompatible with your other one[1]. If users are inattentive and will ignore arbitrary amounts of text, the explanation needs to go in the one place they have to look (the macro name). [1] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-July/330559.html
Quoting Andrew Sayers (2024-07-02 12:13:16) > On Tue, Jul 02, 2024 at 11:52:29AM +0200, Anton Khirnov wrote: > > Quoting Andrew Sayers (2024-07-02 11:08:39) > > > An inattentive user might not see the explanation at the top of this file. > > > Paste the explanation to all the places they might see it. > > > > Duplication is bad, and the premise doesn't work anyway. Inattentive > > users will happily ignore arbitrary amounts of text. In fact the more > > text there is, the more of it they will ignore. > > Meanwhile you make actual information harder to find for people who > > actually bother to read. > > That's a reasonable argument, but incompatible with your other one[1]. > If users are inattentive and will ignore arbitrary amounts of text, > the explanation needs to go in the one place they have to look (the > macro name). I don't understand your point. In my other email I'm objecting to breaking API for flimsy reasons, how is that related to documentation?
On Tue, Jul 02, 2024 at 12:16:24PM +0200, Anton Khirnov wrote: > Quoting Andrew Sayers (2024-07-02 12:13:16) > > On Tue, Jul 02, 2024 at 11:52:29AM +0200, Anton Khirnov wrote: > > > Quoting Andrew Sayers (2024-07-02 11:08:39) > > > > An inattentive user might not see the explanation at the top of this file. > > > > Paste the explanation to all the places they might see it. > > > > > > Duplication is bad, and the premise doesn't work anyway. Inattentive > > > users will happily ignore arbitrary amounts of text. In fact the more > > > text there is, the more of it they will ignore. > > > Meanwhile you make actual information harder to find for people who > > > actually bother to read. > > > > That's a reasonable argument, but incompatible with your other one[1]. > > If users are inattentive and will ignore arbitrary amounts of text, > > the explanation needs to go in the one place they have to look (the > > macro name). > > I don't understand your point. In my other email I'm objecting to > breaking API for flimsy reasons, how is that related to documentation? I could be wrong, but I think this points to a fundamental issue... We normally talk about ABI (binary interface) and API (programming interface), then say "documentation" as if that's some separate thing. It would have been better if programmers had used a term like "ADI" (developer interface) instead, but the world didn't go that way. API is as intermingled with documentation as it is with ABI, and drawing a bright line between the two just causes problems. In this case, spelling it "AV_OPT_FLAG_RUNTIME_PARAM" isn't API, it's documentation. The compiler would work just the same if it had been called e.g. "AV_OPT_FLAG_15", the name is just there for us humans. This patch is about fixing the documentation, so the primary justification is that the old spelling mislead humans. Breaking the API is a side-effect, and I'd argue it's a net benefit, because it nudges external devs to fix their code. You can make the opposite argument, but either way it's incidental to the main goal of picking a spelling that unambiguously documents what the macro does.
diff --git a/libavutil/opt.h b/libavutil/opt.h index b78c3406fa..289ae9f410 100644 --- a/libavutil/opt.h +++ b/libavutil/opt.h @@ -496,6 +496,9 @@ typedef struct AVOptionRanges { /** * Set the values of all AVOption fields to their default values. * + * Note: after a struct is initialized, only options with the + * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified. + * * @param s an AVOption-enabled struct (its first member must be a pointer to AVClass) */ void av_opt_set_defaults(void *s); @@ -505,6 +508,9 @@ void av_opt_set_defaults(void *s); * AVOption fields for which (opt->flags & mask) == flags will have their * default applied to s. * + * Note: after a struct is initialized, only options with the + * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified. + * * @param s an AVOption-enabled struct (its first member must be a pointer to AVClass) * @param mask combination of AV_OPT_FLAG_* * @param flags combination of AV_OPT_FLAG_* @@ -674,6 +680,9 @@ enum { * key. ctx must be an AVClass context, storing is done using * AVOptions. * + * Note: after a struct is initialized, only options with the + * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified. + * * @param opts options string to parse, may be NULL * @param key_val_sep a 0-terminated list of characters used to * separate key from value @@ -692,6 +701,9 @@ int av_set_options_string(void *ctx, const char *opts, * Parse the key-value pairs list in opts. For each key=value pair found, * set the value of the corresponding option in ctx. * + * Note: after a struct is initialized, only options with the + * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified. + * * @param ctx the AVClass object to set options on * @param opts the options string, key-value pairs separated by a * delimiter @@ -722,6 +734,9 @@ int av_opt_set_from_string(void *ctx, const char *opts, /** * Set all the options from a given dictionary on an object. * + * Note: after a struct is initialized, only options with the + * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified. + * * @param obj a struct whose first element is a pointer to AVClass * @param options options to process. This dictionary will be freed and replaced * by a new one containing all options not found in obj. @@ -739,6 +754,9 @@ int av_opt_set_dict(void *obj, struct AVDictionary **options); /** * Set all the options from a given dictionary on an object. * + * Note: after a struct is initialized, only options with the + * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified. + * * @param obj a struct whose first element is a pointer to AVClass * @param options options to process. This dictionary will be freed and replaced * by a new one containing all options not found in obj. @@ -777,6 +795,9 @@ int av_opt_copy(void *dest, const void *src); * @{ * Those functions set the field of obj with the given name to value. * + * Note: after a struct is initialized, only options with the + * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified. + * * @param[in] obj A struct whose first element is a pointer to an AVClass. * @param[in] name the name of the field to set * @param[in] val The value to set. In case of av_opt_set() if the field is not