[Mesa-dev] [PATCH] st/vdpau: fix YCbCr down/up-loads for buffers larger than requested

[Andy Furniss]

Christian König wrote:

>>             Unfortunately it's not quite right, there's an artifact on
>>             r300 s/w
>>             decode = thin green lines right and bottom.
>>             R600 with the patch doesn't show it as much, but 1080
>>             stuff does have
>>             one at the bottom.
>>
>>
> Mhm, that is interesting. Can you start mplayer with gdb and put a
> breakpoint on vlVdpVideoMixerRender and then see what is provided as
> "video_source_rect" by the player? Or just set VDPAU_TRACE=1 and send me
> the output.

Testing on r600. It looks like only vids where width or height is not 
divisible by 16 are affected - I found one that is 1266 wide and that 
ends up with 1px green on the right.

Here's snipped vdpau trace from 1920x1080 which has 1px green at the 
bottom with the patch.

VO: [vdpau] 1920x1080 => 1920x1080 Planar YV12  [fs]
vdp_presentation_queue_target_create_x11(1, 16777217, -)
     -> 0, 2
vdp_presentation_queue_create(1, 2, -)
     -> 0, 3
vdp_presentation_queue_set_background_color(3, {0.010000, 0.020000, 
0.030000, 0.000000})
     -> 0
vdp_video_mixer_create(1, 1, {0}, 3, {0, 1, 2}, {1920, 1080, 0}, -)
     -> 0, 4
vdp_video_mixer_set_feature_enables(4, 1, {0}, {0})
     -> 0
vdp_generate_csc_matrix({(ver=0) 0.000000, 1.000000, 1.000000, 
0.000000}, 0, -)
     -> 0, {{1.164000, 0.000000, 1.596000, -0.874165}, {1.164000, 
-0.391000, -0.813000, 0.531326}, {1.164000, 2.018000, 0.000000, -1.085992}}
vdp_video_mixer_set_attribute_values(4, 1, {1}, {{{1.164000, 0.000000, 
1.596000, -0.874165}, {1.164000, -0.391000, -0.813000, 0.531326}, 
{1.164000, 2.018000, 0.000000, -1.085992}}})
     -> 0
vdp_output_surface_create(1, 0, 1920, 1080, -)
     -> 0, 5
vdp_output_surface_create(1, 0, 1920, 1080, -)
     -> 0, 6
vdp_output_surface_create(1, 0, 1920, 1080, -)
     -> 0, 7
vdp_output_surface_create(1, 0, 1920, 1080, -)
     -> 0, 8
vdp_video_surface_create(1, 0, 1920, 1080, -)
     -> 0, 9
vdp_video_surface_put_bits_y_cb_cr(9, 1, {0xb4424020, 0xb4622020, 
0xb46a1820}, {1920, 960, 960}, )
     -> 0
vdp_presentation_queue_block_until_surface_idle(3, 5, -)
     -> 0, 1339167070715910000
vdp_video_mixer_render(4, 4294967295, NULL, 2, 2, {4294967295, 
4294967295}, 9, 1, {9}, {0, 0, 1920, 1080}, 5, NULL, {0, 0, 1920, 1080}, 
0, NULL)
     -> 0
vdp_presentation_queue_display(3, 5, 1920, 1080, 0)
     -> 0
vdp_video_surface_create(1, 0, 1920, 1080, -)
     -> 0, 10
vdp_video_surface_put_bits_y_cb_cr(10, 1, {0xb3f15020, 0xb3df7020, 
0xb3e77020}, {1920, 960, 960}, )
     -> 0
vdp_presentation_queue_block_until_surface_idle(3, 6, -)
     -> 0, 1339167070715910000
vdp_video_mixer_render(4, 4294967295, NULL, 2, 2, {9, 9}, 10, 1, {10}, 
{0, 0, 1920, 1080}, 6, NULL, {0, 0, 1920, 1080}, 0, NULL)
     -> 0
vdp_presentation_queue_display(3, 6, 1920, 1080, 0)
     -> 0
V:   1.8   3/  3 ??% ??% ??,?% 0 0

>>         Playing some more - enabling npot for r300 will make it the
>>         same as r600
>>         with the patch.
>>
> Yeah, but there was some regression with NPOT+VDPAU, I can't remember
> what it was exactly but that was the reason we added a separate NPOT
> property for video in the first place.

OK. but wasn't that r600 - which is fixed/enabled now anyway.

>>         Never tried 422 -vo on r300 before, but it fails whatever

> r300g doesn't supports that format, but I don't know why it fails with
> that error message, it should just gracefully tell mplayer though VDPAU
> that this isn't supported. Ah, wait a moment, IIRC mplayer never checked
> those properties....

Ahh, I forgot that.

I guess it would be a lot easier all round if the players followed the 
spec. It's seems quite clear about checking buffer sizes.
Xine does check IIRC, but then just uses the info to print a warning 
about segfaults ahead. Mplayer doesn't, I don't think flashplayer does, 
but it doesn't crash because it always seems to ask for video surfaces 
that are a power of two above what it needs anyway.


>
> Sounds like we just hit some bandwidth limit in the combination NPOT+HD
> content.
>
> Don't know what to do about it, the basic problem is that I don't have
> an r3xx system any more, together with my RV670 that stopped working
> when my last AGP based board died.

OK - Speaking of rv670 just before I pulled mine out to test r300 I 
noticed that the problems it had decoding had partially got better when 
color tiling was forced on. It only cured SD though :-(

>> Replied all, I hope it helps.
>> Emeric
> Thanks Emeric, otherwise I wouldn't have noticed Andys mails. The only
> good reason I still use this vodafone address is its good spam filter,
> but that sometimes is to strict. If that happens again just contact me
> on my AMD address (christian.koenig at amd.com).

It seems there was an issue with my ISP server spamming - supposedly 
fixed now and getting de-blacklisted is in progress. I'll CC your new 
address as well to be sure.