rav1e icon indicating copy to clipboard operation
rav1e copied to clipboard

rav1e_config_set_sample_aspect_ratio() from the C api silently accepts unsupported values

Open jamrial opened this issue 1 year ago • 1 comments

The function returns void, meaning no failure is expected, yet it accept values that will be rejected when calling rav1e_context_new() with the provided RaConfig. An example is 0:1 or 0:0. Since rav1e_context_new() returns a pointer on success or NULL on failure, the user has no way to know what was set wrong in the RaConfig, so a function like rav1e_config_set_sample_aspect_ratio() should have been defined to return an error value.

I suggest deprecating rav1e_config_set_sample_aspect_ratio() and introducing a new rav1e_config_set_sar() function or similar that returns an int as replacement. And while at it, the same should probably be done with rav1e_config_set_time_base(). I haven't checked this one, but you can pass values to it that are surely going to be rejected too.

jamrial avatar Oct 15 '22 15:10 jamrial

From rav1e's point of view, it seems easier to deprecate rav1e_context_new() and add another function similar to it, but that returns the possible error in the configuration, maybe like a C enum version of InvalidConfig, but without the inner fields (or maybe try to represent it as a tagged union? not sure what the best way to represent a sum type is in C...).

Would that also work for your use case where you need the exact error?

redzic avatar Oct 15 '22 18:10 redzic