Opened 11 years ago

Closed 11 years ago

#2062 closed defect (fixed)

Playback of HD-PVR recordings corrupted when using VDPAU

Reported by: jyavenard Owned by:
Priority: important Component: avcodec
Version: git-master Keywords: h264 regression
Cc: nfxjfg@googlemail.com Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

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.

Attachments (1)

2062-0.10.patch (1.5 KB ) - added by jyavenard 11 years ago.
Patch for 0.10 (applies for 0.9 with fuzzy)

Download all attachments as: .zip

Change History (12)

comment:1 by gjdfgh, 11 years ago

Cc: nfxjfg@googlemail.com added

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.

comment:2 by jyavenard, 11 years ago

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

comment:3 by Carl Eugen Hoyos, 11 years ago

Keywords: vdpau h264 added

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

comment:4 by jyavenard, 11 years ago

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

comment:5 by Paul Gardiner, 11 years ago

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.

comment:6 by Paul Gardiner, 11 years ago

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

comment:7 by Michael Niedermayer, 11 years ago

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!

comment:8 by jyavenard, 11 years ago

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.

comment:9 by jyavenard, 11 years ago

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

comment:10 by Paul Gardiner, 11 years ago

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.

comment:11 by Carl Eugen Hoyos, 11 years ago

Keywords: regression added; vdpau removed
Priority: normalimportant
Resolution: fixed
Status: newclosed
Version: unspecifiedgit-master

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!

by jyavenard, 11 years ago

Attachment: 2062-0.10.patch added

Patch for 0.10 (applies for 0.9 with fuzzy)

Note: See TracTickets for help on using tickets.