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

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

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.

Comment 1

[Blake Wu [:bwu][:blakewu]]

Alfredo,
Our tolerance for decode errors in bug 1068151 might need to be changed.  
WDYT? Can you help check this bug?

Comment 2

[Jean-Yves Avenard [:jya]]

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

Comment 3

[Alfredo Yang (:alfredo)]

(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?

Comment 4

[Jean-Yves Avenard [:jya]]

That would be good too. Great idea. 

I still think it should be 0 during mochitest even on opt build.

Comment 5

[Alfredo Yang (:alfredo)]

Ok, I'll go for debug build and the mochitest especially for the test like test_playback.html on opt build.

Comment 6

[Alfredo Yang (:alfredo)]

Attachment: Bug 1364872 - take zero tolerance of decoding error in debug and playback test.

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

Comment 7

[Jean-Yves Avenard [:jya]]

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.

Comment 8

[Alfredo Yang (:alfredo)]

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

Comment 9

[Jean-Yves Avenard [:jya]]

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.

Comment 10

[Alfredo Yang (:alfredo)]

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

Comment 11

[Alfredo Yang (:alfredo)]

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1de90f1305e0
take zero tolerance of decoding error in debug and playback test. r=jya

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1de90f1305e0