On Sun, 3 Mar 2024, James Almer wrote:

On 3/3/2024 12:53 PM, Diederick C. Niehorster wrote:
 On Sun, Mar 3, 2024 at 3:55 PM Anton Khirnov <an...@khirnov.net> wrote:

 Quoting Marton Balint (2024-02-26 20:38:46)
 The more I think about it, it is actually a broader problem.

 You are changing the semantics of existing AV_OPT_TYPE_xxx types. So
 previously an option with AV_OPT_TYPE_STRING used to have default value
 in
 default_val.str. After your patch, it will be either default_val.str, or
 default_val.str[1], based on if it is an array or not.

 I think the API user safely assumed that if the option type is known to
 him, he will always find the default value in the relevant default_val
 field. It is actually a bigger issue for an array of AV_OPT_TYPE_INT,
 because previously to get the default value AVOption->default_val.i64
 was
 used, and now .str[1] should be instead...

 In my view the semantics of default_val (and offset) are only defined
 when declaring options on your own object, not when accessing those
 fields when declared by some other code. I also see no good reason for
 any user to read these fields.


 I disagree. Here an example: for a GUI using some part of ffmpeg and
 wanting to give the user some control over the ffmpeg operation, it would
 not be strange to be able to set options and either indicate which values
 are default, or have a "reset to defaults" button. I have written such a
 thing (not yet opensourced).

There's av_opt_set_defaults() to set default values, then av_opt_is_set_to_default() and av_opt_is_set_to_default_by_name() to check if an option is already set to the default value.

What Anton said is that the user has no need to access the fields directly when there are helpers explicitly documented for this purpose.

The AVOption struct is public, and the default_val field is public too. There is nothing that warns the user not to access it. The fact that there are helper functions does not change this.

But it is not the default values that is a problem. The semantic change is the problem. I could have had a code which iterates through every AVOption of an object and prints the values with type AV_OPT_TYPE_INT. Your change will suddently break that, because arrays will also have the type of AV_OPT_TYPE_INT.

So can you please introduce a new option type for arrays?

Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Reply via email to