Bug 98914

Created attachment 128287 [details]
vdpauinfo output

Created attachment 128288 [details]
dmesg output

Price question: Can you bisect?

Is it already broken with 13.0.0, or only with 13.0.1? If the latter, I'm afraid it might be due to https://cgit.freedesktop.org/mesa/mesa/commit/?h=13.0&id=9c297c5487bf5464f75ffd5e11ad6aaad92915a9 .

Meanwhile, I've tried version 13.0.0-1 from the debian snapshot archive. It shows already the same regression for the vdpau hardware acceleration with mpeg2video codec.
I've also cloned the mesa git repository. I'll try to bisect in the next days.

The bisection between mesa-13.0.0 and mesa-12.0.4 leads to

1fb4179f927442354f93dfc8494f0236e50af838 is the first bad commit
commit 1fb4179f927442354f93dfc8494f0236e50af838
Author: Jan Vesely <jan.vesely@rutgers.edu>
Date:   Thu Jun 9 23:01:46 2016 -0400

    vl: Fix trivial sign compare warnings
    
    v2: add whitepace fixes
    
    Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
    Acked-by: Jose Fonseca <jfonseca@vmware.com>
    [Emil Velikov: squash a few more whitespace issues]
    Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

Indeed, reverting just this commit on top of 13.0.0 makes the vdpau hardware video output for the mpeg2video codec work again with r600 on my HP Pavilion dv7.

Good work, looks like this fix had some unintended side effects. 

Please try the following:

Revert the change without creating a commit, e.g. run "git revert -n 1fb4179f927442354f93dfc8494f0236e50af838".

Then reset the changes to their not added state, e.g. run "git reset HEAD". This should give you the following list:
M	src/gallium/auxiliary/vl/vl_deint_filter.c
M	src/gallium/auxiliary/vl/vl_idct.c
M	src/gallium/auxiliary/vl/vl_matrix_filter.c
M	src/gallium/auxiliary/vl/vl_median_filter.c
M	src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
M	src/gallium/auxiliary/vl/vl_vlc.h
M	src/gallium/auxiliary/vl/vl_zscan.c

Now use git checkout on each file (e.g. "git checkout src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c") to figure out what change actually broke the decoding.

My best bet is vl_mpeg12_bitstream.c, but could as well be vl_idct.c, vl_zscan.c or vl_vlc.h.

Thanks for caring and your git lesson.

The culprit is the change in src/gallium/auxiliary/vl/vl_zscan.c .
I double-checked by starting from mesa-13.0.2 and just reverted the type change in that file back to "signed i;" which is enough to repair this regression.

Yeah, the problem is on line 155 in that file.

Here "i" is used as signed and the calculation result can be negative.

Brave enough to create a patch for this or should I do that?

Thanks for the help and it's nice to know that this is still used.

Please, go ahead and commit the patch.

I think there might be many computers which could and do use this hardware acceleration. The mpeg2video codec applies to DVD's for example.
Although IMO, the choice of vdpau hardware support (also for AMD GPUs) by installing the right package (mesa-vdpau-drivers on debian) and the configuration of mpc or vlc is not obvious.

Created attachment 128472 [details] [review]
Possible fix

The reason I've wondered that somebody is still using this is that code is only used by very old hardware generation which don't have native MPEG2 support.

Anyway, please confirm that the attached patch fixes the problem and I will commit it.

Thanks for the patch. I reviewed and tested it ok. The hardware accelerated mpeg2video codec works, e.g., with mpv.

So, my five years old notebook is "very" old :-(.
I did all testing with the integrated GPU (ATI Mobility Radeon HD 4200).
From time to time I try the offloading to the discrete GPU (ATI Mobility Radeon HD 5000 Series) with PRIME. But all too often it hard crashes this notebook.

Just pushed the patch after coming back from vacation.

Should eventually show up in stable releases as well.