[FFmpeg-devel] [PATCH] avfilter/scale*: add option reset_sar
Gyan Doshi
ffmpeg at gyani.pro
Sat Feb 8 07:40:01 EET 2025
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.
>> + at 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
More information about the ffmpeg-devel
mailing list