Re: [FFmpeg-devel] [PATCH] avfilter/scale*: add option reset_sar

[Gyan Doshi <ffmpeg@gyani.pro>]

On 2025-02-07 08:24 pm, Leo Izen wrote:
> On 1/31/25 8:00 AM, Gyan Doshi wrote:
>>
>>
>> diff --git a/doc/filters.texi b/doc/filters.texi
>> index a14c7e7e77..71de1ab2dc 100644
>> --- a/doc/filters.texi
>> +++ b/doc/filters.texi
>> @@ -21285,6 +21285,13 @@ This option can be handy if you need to have 
>> a video fit within or exceed
>>   a defined resolution using @option{force_original_aspect_ratio} but 
>> also have
>>   encoder restrictions on width or height divisibility.
>>   +@item reset_sar
>> +For anamorphic videos, enabling this option leads to adjustment of 
>> output dimensions
>> +to obtain square pixels when the user requests proportional scaling 
>> through either of
>> +the width or height expressions or through force_original_aspect_ratio.
>> +Output SAR is always reset to 1.
>> +Default is false.
>> +
>>   @end table
>
> Documentation should probably state that if 
> force_original_aspect_ratio is set, but width and height expressions 
> are unset, that the width of the input is modified, but not the 
> height. This way a user with, say, a 16:9 rip of a DVD will know that 
> the height will be preserved.

Dimensions are adjusted only if "user requests proportional scaling 
through either of the width or height expressions or through 
force_original_aspect_ratio"

so  'scale=reset_sar=1'  or ' scale=500:500:reset_sar=1'  will lead to 
output of 'inlink->w x inlink->h' and '500 x 500' respectively. Only the 
sample_aspect_ratio is normalized.

I will change the doc to emphasize this better.


>> +    double w_adj = 1;
>
> Nit: you've initialized it as 1.0 everywhere else.

Will change.

>>
>> +    if (scale->reset_sar)
>> +        outlink->sample_aspect_ratio = (AVRational){1, 1};\
>
> This doesn't compile on MSVC (for some reason). use av_make_q(1, 1);

That's surprising. This form is used in many places, including AVFrame 
init and video decode checks. See below.

>> +        w_adj = inlink->sample_aspect_ratio.num ?
>> +        (double)inlink->sample_aspect_ratio.num / 
>> inlink->sample_aspect_ratio.den : 1;
>
> I believe you want to check if the denominator is nonzero here, not 
> the numerator. Otherwise this gives infinity.

This is the standard check in filters, including in this filter. See 
assignment of scale->var_values[VAR_SAR] in scale_eval_dimensions().
The rationale being that the SAR is initialized to 0/1 in lavu/frame.c: 
get_frame_defaults()
and that when decoding, lavc/decode.c: ff_decode_frame_props() ensures 
that den > 0.
Same is the case for other internal assignments or public interfaces. 
So, if numerator is non-zero, denominator is guaranteed to be as well.
See these two functions for examples of AVRational cast assignment.


>>   -    ff_scale_adjust_dimensions(inlink, &vkctx->output_width, 
>> &vkctx->output_height, 0, 1);
>> +    ff_scale_adjust_dimensions(inlink, &vkctx->output_width, 
>> &vkctx->output_height, 0, 1, 1.f);
>>         outlink->w = vkctx->output_width;
>>       outlink->h = vkctx->output_height;
>
>
> Overall looks fine. However, here's one possible issue I forsee 
> happening:
>
> A user has an NTSC DVD which is 720x480 and 16:9 DAR, or 32:27 SAR. 
> That user does something simple like:
>
> ffmpeg -i dvdrip.mpg -vf scale=reset_sar=1 -c:v libx264 ...
>
> Then, it will scale the video to 853x480, which will then fail to 
> encode in libx264 because it requires an even width with yuv420p. 
> Then, the user will change it to 
> scale=reset_sar=1:force_divisible_by=2 and it still won't work because 
> force_original_aspect_ratio is unset.
>
> I don't know if this is possibly a user issue cause the solution here 
> is to use reset_sar=1:force_original_aspect_ratio=1:force_divisible_by=2
>
> In either case, it may be a good idea to automatically enable 
> force_original_aspect_ratio if the width and height expressions are 
> unset, but reset_sar is set.

At present, lack of any dimension specifiers is a no-op in terms of 
dimension change. Not sure it's a good idea to change that.
I can document both in the description as well as an example how a 
simple flattening the picture could be done.

Regards,
Gyan

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