1347538 - Seeking in WebM video causes corruption message

Status: VERIFIED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[684sigma]

Attachment: 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)

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I looked into this, just for fun.

This looks like a regression from https://hg.mozilla.org/mozilla-central/rev/13ce99c3c8af.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 3

[Jean-Yves Avenard [:jya]]

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

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1347538: Don't reject seeks with EOS.

The MediaDecoderStateMachine treat seek's EOS as fatal errors, so instead we always resolve the seek promise, and let the next GetSample return EOS.

Review commit: https://reviewboard.mozilla.org/r/123496/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/123496/

Comment 5

[Jean-Yves Avenard [:jya]]

It will be easier and quicker to always let seek be resolved.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

[Matthew Gregan [:kinetik]]

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

https://reviewboard.mozilla.org/r/123496/#review126510

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e86850951152
Don't reject seeks with EOS. r=kinetik

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e86850951152

Comment 10

[Jean-Yves Avenard [:jya]]

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

Comment 11

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 12

[roxleitan]

Verified fixed using the latest Nightly 55.0a1 (2017-03-29) on Ubuntu 16.04, Mac OS X 10.10 and Windows 10 x64.

Comment 13

[Julien Cristau [:jcristau]]

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

fix webm regression in aurora54

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/697d237e3c00

Comment 15

[Emil Ghitta, QA [:emilghitta]]

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,

Comment 16

[Emil Ghitta, QA [:emilghitta]]

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).