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-requ...@ffmpeg.org with subject "unsubscribe".
