On 09/12/2017 10:24 AM, Emil Velikov wrote:
Hi Leo,

On 12 September 2017 at 14:56, Leo Liu <leo....@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?
The OMX code has been changed. the alternative for "Revert patch OMX" will be followed along with other patches in a set.

This alignment patch will not be committed alone. Most likely Sending it alone is for "RFC", it would be better to add RFC before I sent.

Looks like this should have a a fixes and/or a stable tag.
Yes. It's not currently affecting OMX, since the workaround for OMX, but it affects VA-API scaling for interlaced buffers.



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


"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-sta...@lists.freedesktop.org
"

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

Leo

Emil

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to