Closed Bug 556821 Opened 10 years ago Closed 9 years ago
broken first frame for oggz-choped videos
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a4pre) Gecko/20100402 Minefield/3.7a4pre Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a4pre) Gecko/20100402 Minefield/3.7a4pre new ogg backend in moziall-central displays a broken frame before starting playback i.e. open http://ia331436.us.archive.org/1/items/ulcer_at_work/ulcer_at_work.ogv?ulcer_at_work/ulcer_at_work.ogv&t=123.4 and see broken frame show up shortly. Reproducible: Always
looks like this is still around, any chance to get this fixed? also visible on http://pad.ma videos if you start playing.
The attached video is the first part of the video given in URL. Loading in a <video> element shows a corrupt first frame.
The initial frame in the video is not a keyframe. My initial plan to fix this is to skip any non-keyframes when getting the starttime/finding what we consider to be the first frame of the video.
Initial attempt at a fix. Skips non keyframes at the start of the video. This fixes the issue with the sample video but there are 10 test failures in test_playback where expected durations are wrong. These videos probably don't have keyframes at the start - I'll look into it. Also need to add a test for this case.
Isn't the correct fix for this to honour the presentation time in the Ogg skeleton?
I have no idea. Link to docs on the presentation time in Ogg Skeleton?
Found it: http://wiki.xiph.org/Ogg_Skeleton_4 I think you're right Matthew.
This version of the patch reads the presentation time from the skeleton track and discards any frames before this time. What are your thoughts on this approach? Test video plays fine and all tests pass.
Attachment #514708 - Attachment is obsolete: true
Looks great to me.
This is the previous fix with change for possible zero denominator in the presentation time as noted by cpearce. Added a test video.
Attachment #515543 - Flags: review?(chris) → review+
Rebased to trunk, carried forward review by cpearce.
Oops, this is the real rebased patch. The other one had a missing file.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing post ff4]
Target Milestone: --- → mozilla2.2
Testcase in comment 0 seems to be missing. Testcase in comment 1 works as expected though. VERIFIED FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.