Seeking in WebM video causes corruption message

VERIFIED FIXED in Firefox 54

Status

()

VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: 684sigma, Assigned: jya)

Tracking

({regression})

55 Branch
mozilla55
regression
Points:
---

Firefox Tracking Flags

(firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54 verified, firefox55 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
Created attachment 8847589 [details]
This video shows corruption message, by reporter.webm

I have a problem with Firefox Nightly 55. It doesn't happen in Firefox Beta 53.
When I was watching my discarded video of Bug 1344339, the video occasionally showed message "file is corrupt".
So far I only saw it in one video. One specific scenario when it happens

1. Open attached video
2. Click on the center of timeline

Result: Playback stops. Video shows error message "Video can’t be played because the file is corrupt."
Expected: Video shouldn't stop, because the file is not corrupt.
(* not corrupt enough for Beta 53 to be unable to play it)
(Reporter)

Updated

2 years ago
Has STR: --- → yes
Keywords: regression
I looked into this, just for fun.

This looks like a regression from https://hg.mozilla.org/mozilla-central/rev/13ce99c3c8af.
The WebM file at issue here says it has two tracks (video, audio) but it contains only video data. So during WebMTrackDemuxer::Seek() for the audio track, after seeking, WebMDemuxer::NextPacket gets called to find the next audio packet, but it doesn't find one before the end-of-stream, so it bails out with a seek error which eventually gets treated as a corrupt file. Before 13ce99c3c8af, WebMTrackDemuxer::Seek would basically ignore failure of GetNextPacket.

This WebM file may or may not be valid, but Chrome can play and seek in it without error.
(Assignee)

Updated

2 years ago
Flags: needinfo?(jyavenard)
(Assignee)

Comment 3

2 years ago
The behaviour of the WebMDemuxer + MediaFormatReader appears correct to me, the seek promise is rejected with END_OF_STREAM.

The MDSM however doesn't handle this case and cause the media element to enter in error mode.

Not sure on the best approach here:

Work around the issue in WebMDemuxer, have the seek completes but in a dummy fashion, the next GetSample will return EOS.
Or fix the MDSM so that it correctly handles EOS during seeking.

JW, what do you think?

Jean-Yves
Assignee: nobody → jyavenard
Flags: needinfo?(jyavenard) → needinfo?(jwwang)
(Assignee)

Updated

2 years ago
Blocks: 1334508
Comment hidden (mozreview-request)
(Assignee)

Comment 5

2 years ago
It will be easier and quicker to always let seek be resolved.
Flags: needinfo?(jwwang)
Given that seeking in this file works in Chrome and used to work in Firefox, seems to me that it should keep working.

Comment 7

2 years ago
mozreview-review
Comment on attachment 8851049 [details]
Bug 1347538: Don't reject seeks with EOS.

https://reviewboard.mozilla.org/r/123496/#review126510
Attachment #8851049 - Flags: review?(kinetik) → review+

Comment 8

2 years ago
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e86850951152
Don't reject seeks with EOS. r=kinetik

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e86850951152
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Updated

2 years ago
status-firefox54: --- → affected
(Assignee)

Comment 10

2 years ago
Comment on attachment 8851049 [details]
Bug 1347538: Don't reject seeks with EOS.

Approval Request Comment
[Feature/Bug causing the regression]: 1334508
[User impact if declined]: Some videos no longer play
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: by myself
[Needs manual test from QE? If yes, steps to reproduce]: Yes, instructions provided in first description
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: we handle an error later in the timeline
[String changes made/needed]: none
Attachment #8851049 - Flags: approval-mozilla-aurora?
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected
status-firefox-esr52: --- → unaffected
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: qe-verify+
Flags: needinfo?(brindusa.tot)
Verified fixed using the latest Nightly 55.0a1 (2017-03-29) on Ubuntu 16.04, Mac OS X 10.10 and Windows 10 x64.
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
Flags: needinfo?(brindusa.tot)
Comment on attachment 8851049 [details]
Bug 1347538: Don't reject seeks with EOS.

fix webm regression in aurora54
Attachment #8851049 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 14

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/697d237e3c00
status-firefox54: affected → fixed
I’ve reproduced the issue described in comment 0 using Firefox 55.0a1 (Build Id:20170314110401) and verified that the issue is not reproducible anymore using Firefox 54.0b1 (Build Id:20170420081804) on Ubuntu 16.04 64bit,
status-firefox54: fixed → verified
Flags: qe-verify+
It seems that I forgot to mention that this issue was also verified on Windows 10 64bit and macOS 10.11.6. (Sorry for the mistake).
You need to log in before you can comment on or make changes to this bug.