Bug 98914 - mesa-vdpau-drivers: breaks vdpau for mpeg2video
Summary: mesa-vdpau-drivers: breaks vdpau for mpeg2video
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/r600 (show other bugs)
Version: 13.0
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Default DRI bug account
QA Contact: Default DRI bug account
Depends on:
Reported: 2016-11-30 15:09 UTC by Jörg-Volker Peetz
Modified: 2017-01-03 11:19 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

X log file (34.12 KB, text/plain)
2016-11-30 15:09 UTC, Jörg-Volker Peetz
vdpauinfo output (3.86 KB, text/plain)
2016-11-30 15:11 UTC, Jörg-Volker Peetz
dmesg output (69.06 KB, text/plain)
2016-11-30 15:12 UTC, Jörg-Volker Peetz
Possible fix (1.42 KB, patch)
2016-12-14 14:06 UTC, Christian König
Details | Splinter Review

Description Jörg-Volker Peetz 2016-11-30 15:09:54 UTC
Created attachment 128286 [details]
X log file

On my AMD CPU/AMD GPU hybrid HP Pavilion notebook under X with radeon video
driver the vdpau hardware video output for the mpeg2video codec broke beginning
with Mesa 13.0. Such videos are totally scrambled on the screen now.
Codec h264 is still working.
This is a debian testing/sid system with custom kernel 4.8.11.
Comment 1 Jörg-Volker Peetz 2016-11-30 15:11:20 UTC
Created attachment 128287 [details]
vdpauinfo output
Comment 2 Jörg-Volker Peetz 2016-11-30 15:12:07 UTC
Created attachment 128288 [details]
dmesg output
Comment 3 Christian König 2016-11-30 15:22:28 UTC
Price question: Can you bisect?
Comment 4 Michel Dänzer 2016-12-01 07:52:12 UTC
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 .
Comment 5 Jörg-Volker Peetz 2016-12-01 12:23:59 UTC
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.
Comment 6 Jörg-Volker Peetz 2016-12-08 11:31:13 UTC
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.
Comment 7 Christian König 2016-12-08 13:26:03 UTC
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.
Comment 8 Jörg-Volker Peetz 2016-12-08 14:46:46 UTC
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.
Comment 9 Christian König 2016-12-08 15:36:42 UTC
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.
Comment 10 Jörg-Volker Peetz 2016-12-08 16:34:19 UTC
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.
Comment 11 Christian König 2016-12-14 14:06:41 UTC
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.
Comment 12 Jörg-Volker Peetz 2016-12-14 15:09:07 UTC
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.
Comment 13 Christian König 2017-01-03 11:19:55 UTC
Just pushed the patch after coming back from vacation.

Should eventually show up in stable releases as well.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.