[Mesa-dev] [PATCH] radeonsi/uvd: fix interlaced video buffer height alignment

[Emil Velikov]

Hi Leo,

On 12 September 2017 at 14:56, Leo Liu <leo.liu at amd.com> wrote:
> In code:
>         template.height = align(tmpl->height / array_size, VL_MACROBLOCK_HEIGHT);
>         ...
>         template.height *= array_size;
>
> It turns out the height will be aligned with 2*VL_MACROBLOCK_HEIGHT.
> The problematic case for example is when VA-API postproc scaling with
> blit between interlaced buffers, if the size is 720 in height, it will
> be actually scaled to 736 in height, so the scaled video will crop out
> the extra 16 lines of height.
>
> Another example is when deint with 720p video from interlaced buffer
> to progressive buffer. This problem happened on OMX, and got workaround
> with patch:
>
> (0c374a777 st/omx/dec: set dst rect to match src size
>
With this patch in, can/should we revert this patch?
Looks like this should have a a fixes and/or a stable tag.

> When creating interlaced video buffer, hegith set to "template.height =
> align(tmpl->height/ array_size, VL_MACROBLOCK_HEIGHT);", and we use
> "template.height *= array_size;" for the buffer height, so it actually
> aligned with 32. With progressive video buffer it still aligned with 16,
> thus causing different height between interlaced buffer and progressive
> buffer for 4K (height=2160), and 720p (height=720).
>
> When transcode the video, this will cause the 16 lines corruption
> at the bottom of the encode video.)
>
Humble suggestion for alternative, simpler, commit message.

"The current code (snippet below), incorrectly calculates the height
of the video.
Namely: the alignment happens before the division, thus the height
will effectively be aligned to array_size*VL_MACROBLOCK_HEIGHT.

   template.height = align(tmpl->height / array_size, VL_MACROBLOCK_HEIGHT);

So when array_size != 1 (aka interleaved buffer) the resulting height is wrong.
For example: when resolutions is 720, we'll end up with 736.

Adjust the order to align first and then divide.

Note: earlier commit 0c374a777 ("st/omx/dec: set dst rect to match src
size") worked around this issue.
With this fix in, we can remove (XXX: or keep) it because $REASON.

Fixes: ???
Cc: mesa-stable at lists.freedesktop.org
"

With the above,
Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>

Emil