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

[Rémi Denis-Courmont]

Le samedi 22 février 2014, 15:14:25 Felix Abecassis a écrit :
> 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.

Well, it's good but it's not great. The transform filter should follow the same 
padding rules as the VLC core with regards to the scan line pitch. 1090 is a 
really bad choice; it is not even a multiple of 4, forget 16 or 32.

Of course, that is only an optimization (and it might get automagically fixed 
when rotations are properly implemented in outputs and filters).

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

Well ideally the filter chain would be created *before* the video output, so 
that we create artificial format mismatches like that. But OK, patch 1/3 is 
probably fine for the time being.

(...)
> 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.

Well I don't really care about patch 2/3. But patch 3/3 looks very wrong to 
me.

-- 
Реми Денис-Курмон
http://www.remlab.net/