Closed Bug 556821 Opened 10 years ago Closed 9 years ago

broken first frame for oggz-choped videos

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla5

People

(Reporter: j, Assigned: cajbir)

References

()

Details

Attachments

(2 files, 4 obsolete files)

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
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Assignee: nobody → chris.double
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.
Attached patch Initial fix attempt (obsolete) — Splinter Review
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.
Attached patch Another fix idea (obsolete) — Splinter Review
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
OS: Linux → All
Hardware: x86 → All
Looks great to me.
Attached patch Fix + test (obsolete) — Splinter Review
This is the previous fix with change for possible zero denominator in the presentation time as noted by cpearce. Added a test video.
Attachment #514991 - Attachment is obsolete: true
Attachment #515543 - Flags: review?(chris)
Status: NEW → ASSIGNED
Attachment #515543 - Flags: review?(chris) → review+
Whiteboard: [needs landing post ff4]
Attached patch Fix + test (obsolete) — Splinter Review
Rebased to trunk, carried forward review by cpearce.
Attachment #515543 - Attachment is obsolete: true
Attachment #521371 - Flags: review+
Attached patch Fix + testSplinter Review
Oops, this is the real rebased patch. The other one had a missing file.
Attachment #521371 - Attachment is obsolete: true
Attachment #521381 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/4e2e0fd57de4
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.