[FFmpeg-devel] Ensure scaled video is divisible by n

This patch adds a new option to the scale filter which ensures that the
output resolution is divisible by the given integer similar to using -n
in the `w` and `h` options. But this works even if the
`force_original_aspect_ratio` is used.

The use case for this is to set a fixed target resolution using `w` and
`h`, to use the `force_original_aspect_ratio` option to make sure that
the video always fits in the defined bounding box regardless of aspect
ratio, but to also make sure that the calculated output resolution is
divisible by n so in can be encoded with certain encoders/options if
that is required.

Signed-off-by: Lars Kiesow <lkiesow@uos.de>
---
 doc/filters.texi       | 5 +++++
 libavfilter/vf_scale.c | 9 ++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

On Wed, Jul 03, 2019 at 06:16:51PM +0200, Lars Kiesow wrote:
> This patch adds a new option to the scale filter which ensures that the
> output resolution is divisible by the given integer similar to using -n
> in the `w` and `h` options. But this works even if the
> `force_original_aspect_ratio` is used.
> 
> The use case for this is to set a fixed target resolution using `w` and
> `h`, to use the `force_original_aspect_ratio` option to make sure that
> the video always fits in the defined bounding box regardless of aspect
> ratio, but to also make sure that the calculated output resolution is
> divisible by n so in can be encoded with certain encoders/options if
> that is required.
> 
> Signed-off-by: Lars Kiesow <lkiesow@uos.de>
> ---
>  doc/filters.texi       | 5 +++++
>  libavfilter/vf_scale.c | 9 ++++++---
>  2 files changed, 11 insertions(+), 3 deletions(-)

commit message should begin with a prefix like 
avfilter/vf_scale: ...

> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 700a76f239..1694fdda28 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -15215,6 +15215,11 @@ Please note that this is a different thing than specifying -1 for @option{w}
>  or @option{h}, you still need to specify the output resolution for this option
>  to work.
>  
> +@item force_divisible_by
> +Ensures that the output resolution is divisible by the given integer similar
> +to using -n in the @option{w} and @option{h} options. But this works even if
> +the @option{force_original_aspect_ratio} is used.
> +
>  @end table
>  
>  The values of the @option{w} and @option{h} options are expressions
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index f741419e7e..d1b486f3d6 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -86,6 +86,7 @@ typedef struct ScaleContext {
>      int in_v_chr_pos;
>  
>      int force_original_aspect_ratio;
> +    int force_divisible_by;
>  
>      int nb_slices;
>  
> @@ -237,10 +238,11 @@ static int config_props(AVFilterLink *outlink)
>          goto fail;
>  
>      /* Note that force_original_aspect_ratio may overwrite the previous set
> -     * dimensions so that it is not divisible by the set factors anymore. */
> +     * dimensions so that it is not divisible by the set factors anymore
> +     * unless force_divisible_by is defined as well */
>      if (scale->force_original_aspect_ratio) {
> -        int tmp_w = av_rescale(h, inlink->w, inlink->h);
> -        int tmp_h = av_rescale(w, inlink->h, inlink->w);
> +        int tmp_w = av_rescale(h, inlink->w, inlink->h) / scale->force_divisible_by * scale->force_divisible_by;
> +        int tmp_h = av_rescale(w, inlink->h, inlink->w) / scale->force_divisible_by * scale->force_divisible_by;

what does this feature have to do with force_original_aspect_ratio ?
i think it should not be under this if()

also the rounding is always down, it probably should be rounding to closest
(for n=2 it doesnt matter but for larger divisibility like 16 rounding down
by 15 instead of rounding up by 1 seems not ideal


[...]

thx

Hi Michael,

> commit message should begin with a prefix like 
> avfilter/vf_scale: ...

Thanks for the hint. Will use that.

> what does this feature have to do with force_original_aspect_ratio ?
> i think it should not be under this if()

This really only makes sense when using force_original_aspect_ratio
since otherwise, you could just use something like scale=320:-2. The
force_original_aspect_ratio option is great to have a video fit within
a given maximum size but unfortunately, this may cause encoder issues
due to the resulting size.

But I guess that my documentation for this option wasn't the best. I'll
improve that so that it will hopefully become clear what the new option
is meant for.

> also the rounding is always down, it probably should be rounding to
> closest (for n=2 it doesnt matter but for larger divisibility like 16
> rounding down by 15 instead of rounding up by 1 seems not ideal

Good point. Thinking about this again, since I'm coupling this with
force_original_aspect_ratio anyway, it would definitely make sense to
have this comply to the `increase` or `decrease` options of that
setting, rounding up if `increase` is set, rounding down if `decrease`
is set.

I will update the code accordingly and send out a new patch in a moment.

Best regards,
Lars