Closed Bug 1364872 Opened 3 years ago Closed 3 years ago

Errors shouldn't be ignored in mochitest except very specific cases

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jya, Assigned: ayang)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1361984 introduce a serious regression that broke YouTube on Windows.

Two problems occurred:
1- The issue could have been easily identified by manually testing the change.
2- regression tests also didn't find any problem either.

Unfortunately, such manual testing didn't occur, that made us relying on our regression tests to work as intended.

The issue didn't occur because by default, our media player ignores any errors and skip over them until it find something. While it doesn't cause an error, there are usually visually noticeable problems.

media.audio-max-decode-error and media.audio-max-decode-error should be set to 0 by default with the exception of a very particular mochitest that checks that those preferences do work as intended.

A decode error of any of our mochitest media files should yield to a test failure.
Blocks: 1366195
Alfredo,
Our tolerance for decode errors in bug 1068151 might need to be changed.  
WDYT? Can you help check this bug?
Flags: needinfo?(ayang)
See Also: → 1068151
I think they should be set to 0 during mochitest, except for the test that ensure we skip over errors if there's one (I don't see any which is a shame).
See Also: → 1366362
Assignee: nobody → ayang
Flags: needinfo?(ayang)
(In reply to Jean-Yves Avenard [:jya] from comment #2)
> I think they should be set to 0 during mochitest, except for the test that

Or always set to zero in debug?
That would be good too. Great idea. 

I still think it should be 0 during mochitest even on opt build.
Ok, I'll go for debug build and the mochitest especially for the test like test_playback.html on opt build.
Comment on attachment 8871194 [details]
Bug 1364872 - take zero tolerance of decoding error in debug and playback test.

https://reviewboard.mozilla.org/r/142680/#review146378

please add a mochitest with a file that does produce a decode error, to ensure that this functionality does wwork when we want to.
Attachment #8871194 - Flags: review?(jyavenard) → review+
(In reply to Jean-Yves Avenard [:jya] from comment #7)
> Comment on attachment 8871194 [details]
> Bug 1364872 - take zero tolerance of decoding error in debug and playback
> test.
> 
> https://reviewboard.mozilla.org/r/142680/#review146378
> 
> please add a mochitest with a file that does produce a decode error, to
> ensure that this functionality does wwork when we want to.

I tried before, the test file is too large to be in gecko. I also tried to produce a problem video but it decodes to small mosaic artifical frames instead of errors.

So far I found is audio file in bug 1270748 but it doesn't cover video playback.
Why didn't you commit bug 1270748 ?

For generating a broken video, you could use MSE and modify the buffer with rubbish so that it can be demuxed but the decoder will choke on it.
(In reply to Jean-Yves Avenard [:jya] from comment #9)
> Why didn't you commit bug 1270748 ?
> 
> For generating a broken video, you could use MSE and modify the buffer with
> rubbish so that it can be demuxed but the decoder will choke on it.

Because I want to find another video testcase, not just audio. I'll try MSE for that.
hmm... try fails at browser_styleeditor_media_sidebar.js but I really don't think it is my fault.


https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a1ee3b5c0c73356ac719fb58818a6d6c3e7131e&selectedJob=102741961
Pushed by ayang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1de90f1305e0
take zero tolerance of decoding error in debug and playback test. r=jya
https://hg.mozilla.org/mozilla-central/rev/1de90f1305e0
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.