[vlc-devel] [PATCH 3/3] swscale: scale the aspect-ratio of the output format

[Felix Abecassis]

Let me explain the situation regarding this patch set.

In the following I will write the dimensions of a picture as
i_visible_width x i_visible_height (i_width x i_height).

This patch serie is related to 454654e2 and 5b7f672c when I modified
swscale to use the visible resolution of the video instead of the
"hidden" resolution when scaling. Using the hidden resolution caused
swscale to interpolate pixels of the output image using uninitialized
padding data (for instance added by avcodec). We had large green lines
on some videos and with many thumbnails. However I now realize these
patches also broke rotation as reported by
https://trac.videolan.org/vlc/ticket/10745

Let's take as an example a 1080p sample such as in_to_tree_1080p. We
want to rotate it by 90 degrees, we use the following arguments:
--video-filter=transform --transform=90
We use avcodec direct rendering so the pictures have extra lines for
padding, dimension is 1920x1080 (1920x1090).
The "transform" module is correctly handling hidden/visible dimensions
as you said in 1/3, output format of transform filter is 1080x1920
(1090x1920). So far so good.
After the transform filter was added to the chain, we have the
following call: VideoFormatCopyCropAr(&fmt_target.video,
&fmt_current.video);
fmt_target is the initial format: 1920x1080 (1920x1090) and
fmt_current is the output format of the last filter: 1080x1920
(1090x1920).
Function VideoFormatCopyCropAr executes the following statements:
p_dst->i_visible_width  = p_src->i_visible_width;
p_dst->i_visible_height = p_src->i_visible_height;

Variable fmt_target is now: 1080x1920 (1920x1090), which doesn't make
sense at all unless I'm missing something here. This is why I've
deleted the call to VideoFormatCopyCropAr; but we still need to copy
the AR for instance when rotating an anamorphic sample.

After this call we check if we need to add a new filter in the
pipeline to adapt to fmt_target:
if (!es_format_IsSimilar(&fmt_current, &fmt_target)) {
    msg_Dbg(vout, "Adding a filter to compensate for format changes");
    if (!filter_chain_AppendFilter(vout->p->filter.chain_interactive,
NULL, NULL,
                                   &fmt_current, &fmt_target)) {

I think we need an additional filter here because otherwise allocation
of the output picture for transform will go through the vout pool,
which is 1920x1080 and not 1080x1920.
This is when swscale is picked, in our case we have:
swscale debug: 1080x1920 (1090x1920) chroma: I420 -> 1080x1920
(1920x1090) chroma: I420 with scaling using Bicubic (good quality)
The output format is wrong, output picture is garbage.

When swscale was using hidden dimensions it worked because scaling was
1090x1920 -> 1920x1090.
With patch 1/3 we remove this overriding of visible dimension, we now
have a scaling like before:
swscale debug: 1080x1920 (1090x1920) chroma: I420 -> 1920x1080
(1920x1090) chroma: I420 with scaling using Bicubic (good quality)
This is now correct, but because of the scaling the picture is
stretched horizontally and compressed vertically:
http://imgur.com/RNbkjK5. This is not what we expect from a 90 degrees
rotation.

The way to correct this is to set the aspect-ratio of the output
format so that the image is properly resized by the vout, with the
good aspect ratio we now get the expected image:
http://imgur.com/5MCm8DU
This looks hacky and inefficient but it was done this way before, my
patch on visible dimensions broke this feature. Patch 3/3 is
correcting this.

Concerning patch 2/3, I've realized it can actually be done
differently by slightly modifying 3/3, so we won't need it anymore.

Thank you

2014-02-21 20:13 GMT+01:00 Rémi Denis-Courmont <remi at remlab.net>:
> Le vendredi 21 février 2014, 19:49:40 Felix Abecassis a écrit :
>> For instance, when using swscale after a 90 degrees rotation on 1080p,
>> the transformation is 1080x1920 -> 1920x1080. The aspect-ratio must be
>> modified in order to have the displayed image correctly rotated and
>> not stretched.
>
> I think conversion filters are not generally allowed to modify their input and
> output formats (This is still a bit messy in the case of video though.),
> unlike normal/proper filters. This is definitely not safe for non-zero priority
> plugins.
>
> In other words, the aspect ought to be already correct already when entering
> swscale. If it is not, there is probably a bug somewhere else.
>
>> Fix #10745.
>> ---
>>  modules/video_chroma/swscale.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/modules/video_chroma/swscale.c b/modules/video_chroma/swscale.c
>> index 11a8ecb..a22e608 100644
>> --- a/modules/video_chroma/swscale.c
>> +++ b/modules/video_chroma/swscale.c
>> @@ -441,6 +441,16 @@ static int Init( filter_t *p_filter )
>>      p_sys->b_swap_uvi = cfg.b_swap_uvi;
>>      p_sys->b_swap_uvo = cfg.b_swap_uvo;
>>
>> +    /* Compute aspect-ratio of the output format. */
>> +    p_fmto->i_sar_num *= p_fmti->i_visible_width;
>> +    p_fmto->i_sar_den *= p_fmto->i_visible_width;
>> +    vlc_ureduce(&p_fmto->i_sar_num, &p_fmto->i_sar_den,
>> +                p_fmto->i_sar_num, p_fmto->i_sar_den, 65536);
>> +    p_fmto->i_sar_num *= p_fmto->i_visible_height;
>> +    p_fmto->i_sar_den *= p_fmti->i_visible_height;
>> +    vlc_ureduce(&p_fmto->i_sar_num, &p_fmto->i_sar_den,
>> +                p_fmto->i_sar_num, p_fmto->i_sar_den, 65536);
>> +
>>  #if 0
>>      msg_Dbg( p_filter, "%ix%i (%ix%i) chroma: %4.4s -> %ix%i (%ix%i)
>> chroma: %4.4s extend by %d", p_fmti->i_visible_width,
>> p_fmti->i_visible_height, p_fmti->i_width, p_fmti->i_height, (char
>> *)&p_fmti->i_chroma,
>
> --
> Реми Денис-Курмон
> http://www.remlab.net/
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel



-- 
Félix Abecassis
http://felix.abecassis.me