Playback of HD-PVR recordings corrupted when using VDPAU

[jyavenard]

Summary of the bug:
In reference to bug #2050

The following clip:

​http://www.avenard.org/files/media/mediatest/11159%20HD-PVR%20sample.mpg

Presents video corruptions when using VDPAU, where playback devolves into a weird backward-and-forward stuttering, with frames being displayed out of order

git bisect point bfca35114ae174bbabd88526379d95d4022a98b1 as the commit introducing the regression:

commit bfca35114ae174bbabd88526379d95d4022a98b1
Author: Michael Niedermayer <michaelni@gmx.at>
Date:   Wed Nov 16 23:58:06 2011 +0100

    h264: rewrite has_b_frame calculation code,
    the previous implementation was too buggy.
    
    Signed-off-by: Michael Niedermayer <michaelni@gmx.at>

Reversal of that commit against release/1.0 is available at:
​http://pastebin.com/mPqMzCVq

Issue also occurs on master.

[gjdfgh]

Interestingly this actually works in mplayer2 with vdpau hardware decoding (except for some spam at the start of the file, which happens with software decoding too).

Tested with ffmpeg 0b980e57ace0acf74bee8bcfc89cbd4f0ff2a602.

[jyavenard]

The issue show in mythtv (bug report: ​http://code.mythtv.org/trac/ticket/11159)

[Carl Eugen Hoyos]

The sample plays fine with mplayer -vc ffh264vdpau here (with -demuxer mpegts and -demuxer lavf), could you also test on your hardware?

[jyavenard]

Actually, it seems that the main culprit is the rendering playback. When using the VDPAU rendering or OpenGL (in mythtv), the frame mis-order occurs.

it plays okay with mplayer (svn r35710) :(

with:
mplayer -vo vdpau -vc ffh264vdpau -demuxer lavf /data/videos/testing/11159\ HD-PVR\ sample.mpg
or:
mplayer -vo vdpau -vc ffh264vdpau -demuxer mpegts /data/videos/testing/11159\ HD-PVR\ sample.mpg

[Paul Gardiner]

I have a fix for this. Like jyavenard, I have swapped away from Michael Niedermeyer's has_b_frames calculation to use the qatar version. I've then corrected the qatar algorithm so that it handles BBC HD recordings correctly. The two steps of that fix are on the fixes/0.26 branch of my mythtv github fork: ​https://github.com/Glidos/mythtv/commits/fixes/0.26

To be honest, I think Michael's algorithm is the superior: it is simpler, more logical and faster acting (if I understand it correctly), but I believe it is less resiliant to sudden long backward steps in poc (albeit that these happen only in samples that don't conform to the H264 spec). I need to confirm this, but it true, I may be able to add the same resiliance to Michael's algorithm.

The correction to the qatar algorithm is based on its monitoring poc values coming OUT of the reordering process. Because of that, any required delay detected needs to be added to the current delay, not max'd. Also, to avoid multiple unnecessary delay increments, the recored of output pocs needs resetting each time has_b_frames increases.

[Paul Gardiner]

Got it. I can see now why the qatar algorithm is more resilient
to broken files than Michael's, and it wasn't hard to add an
equivalent check to Michael's. It's on my hd_pvr_fix branch
here: ​https://github.com/Glidos/mythtv/commit/86a14fb7a58fa96cbabbf99c23c450a79d8d885f

[Michael Niedermayer]

The code in the suggested patch which clears the last_pocs also triggers for several reference H264 streams. This doesnt seem intended.
Thus fixed differently (in git master), please test!

[jyavenard]

Michael, good news. I backported this to ffmpeg 1.0 (used in mythtv) and I can't reproduce the issue anymore.

Thanks !

And thanks to Paul for finding the problem.

[jyavenard]

Will this fix be backported to both ffmpeg 1.0 and 1.1 ?

[Paul Gardiner]

Yes nice. Indeed that avoids the large has_b_frames value occuring during playback of the HDPVR sample I've been using for testing. So you already had places where inconsistencies were detected, and now you are resetting last_pocs there. My only remaining concern is that the tests don't (seem) to target the specific case of a large backwards transition in poc, and I don't know how to reason that that happening would always trigger also the tests you have in place. So I wonder if it will fix some hdpvr samples and not others.

Concerning the mistriggering you saw when trying my patch, as you say it was unintended, but it was benign. The unnecessary case is when a poc is first entered into an empty last_pocs list (all INT_MIN, that is). It's benign because in that case, both branches of the 'if' lead to the same final program state. I've updated my patch to avoid triggering in that case: ​https://github.com/Glidos/mythtv/commit/f9579b5982cfc7a18c189ed62ddf23eb73d95825, although I don't expect us to need it now we have yours.

[Carl Eugen Hoyos]

I backported the fix to 0.11, 1.0 and 1.1, if somebody needs it in 0.10 (and 0.9): Please send a patch!

[jyavenard]

Patch for 0.10 (applies for 0.9 with fuzzy)