[Mesa-dev] [PATCH] vl/compositor: fix weave shader bugs

[Christian König]

On 18.12.2012 13:21, Maarten Lankhorst wrote:
> Op 18-12-12 13:04, Maarten Lankhorst schreef:
>> Op 18-12-12 11:42, Christian König schreef:
>>> On 18.12.2012 09:18, Maarten Lankhorst wrote:
>>>> Writemask was XY instead of YZ (thanks to calim for spotting it).
>>>>
>>>> The pixel calculation resulted in the pixel always being off by one.
>>>> If y was .5:
>>>>
>>>> y' = round(y) + 0.5 = 1.5
>>>>
>>>> Fixing this also means the LRP function has to swap the pixels it, since
>>>> it's now the other way around for top/bottom.
>>>>
>>>> WIth these fixes only chroma for top and bottom pixel rows are wrongly interpolated
>>>> in my test program:
>>> Sounds good, but what's the reason that first and last row is interpolated wrong? Just some issues with the border color/wrap handling?
>> If I had to guess it interpolates the first and last row, rather than using the exact value of that line for the top/bottom chroma samples
>> there. So it's just border handling sampling from both surfaces that's wrong I think.
>>
>> As you can see in the diff below, it repeats the cbcr[1] value for cbcr[0], similar for cbcr[63] and cbcr[62]:
>>>> --- nvidia
>>>> +++ nouveau
>>>> @@ -1,4 +1,4 @@
>>>> -YCbCr[0] = 00c080
>>>> +YCbCr[0] = 00b070
>>>>    YCbCr[1] = 00b070
>>>>    YCbCr[2] = 029050
>>>>    YCbCr[3] = 207050
>>>> @@ -61,4 +61,4 @@
>>>>    YCbCr[60] = 0c5070
>>>>    YCbCr[61] = c05090
>>>>    YCbCr[62] = 0e70b0
>>>> -YCbCr[63] = e080c0
>>>> +YCbCr[63] = e070b0
>>>>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
>>> Reviewed-by: Christian König <christian.koenig at amd.com>
>> Thanks, will commit.
>>
>> The program I used for testing is attached.
> Now it is!
Thanks and wow, I always wanted to hack together such a test program for 
VDPAU, but never found time to do so. And now your diff above makes a 
bit more sense to me :)

For the correct interpolation on the first/last line you probably need 
just another adjustment on the LRP factor, shouldn't be to hard to hack, 
but at least I don't really have time for that.

>> Also I seem to be getting a lockup if I convert  that program to use YV12 format instead, nouveau requires the interlaced buffer handling for vdec,
>> and disables support for progressive video buffers. Would probably also need the separate patch, for fixing up vl_video_buffer_surfaces.
Well, as said before (depending a bit on the codec and generation I 
think) AMD hardware can do both, but I already added some flags 
indicating to the state tracker what the driver can do and prefers to 
have. Shouldn't be to hard to just return the right values there.

Regarding the order for the interlaced surfaces: I can live with any 
implementation. IIRC I changed that while it was in development, maybe I 
have missed the use in the VDPAU state tracker.

Christian.