1345756 - Can't seek to last audio frame on OS X treeherder

Status: VERIFIED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

Noticed the problem after seeing timeout in a newly test added..

Like here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6594540ac0bb196744ce6869535282a37f622cc

From the logs, it appears that on the 10.10 treeherder test, we can't seek to the last two audio frames, the decoder returns nothing, only after the 3rd frame input will the decoder return something...

I can't reproduce it locally, and what's even weirder is that it appears to work on debug build for some reasons...

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1345756: P1. Add mochitest.

testing seeking to last audio frame buffered.

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

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1345756: P2. Fix coding style.

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

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1345756: P3. Fix coding style.

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

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1345756: [MSE] P4. Pre-roll AAC by 2112 audio frames.

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

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8845320 [details]
Bug 1345756: P1. Add mochitest.

https://reviewboard.mozilla.org/r/118502/#review120432

Comment 6

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8845321 [details]
Bug 1345756: P2. Fix coding style.

https://reviewboard.mozilla.org/r/118504/#review120436

Comment 7

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8845322 [details]
Bug 1345756: P3. Fix coding style.

https://reviewboard.mozilla.org/r/118506/#review120438

Comment 8

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8845323 [details]
Bug 1345756: [MSE] P4. Pre-roll AAC by 2112 audio frames.

https://reviewboard.mozilla.org/r/118508/#review120440

Comment 9

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 5b5b32b145a9 -d 7368729b7ba0: rebasing 381160:5b5b32b145a9 "Bug 1345756: P1. Add mochitest. r=gerald"
merging dom/media/mediasource/test/mochitest.ini
rebasing 381161:7e168336ad94 "Bug 1345756: P2. Fix coding style. r=gerald"
rebasing 381162:a2000a89f9e4 "Bug 1345756: P3. Fix coding style. r=gerald"
merging dom/media/mediasource/MediaSourceDemuxer.cpp
warning: conflicts while merging dom/media/mediasource/MediaSourceDemuxer.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 10

[Jean-Yves Avenard [:jya]]

Comment on attachment 8845320 [details]
Bug 1345756: P1. Add mochitest.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118502/diff/1-2/

Comment 11

[Jean-Yves Avenard [:jya]]

Comment on attachment 8845321 [details]
Bug 1345756: P2. Fix coding style.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118504/diff/1-2/

Comment 12

[Jean-Yves Avenard [:jya]]

Comment on attachment 8845322 [details]
Bug 1345756: P3. Fix coding style.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118506/diff/1-2/

Comment 13

[Jean-Yves Avenard [:jya]]

Comment on attachment 8845323 [details]
Bug 1345756: [MSE] P4. Pre-roll AAC by 2112 audio frames.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118508/diff/1-2/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd4999a68867
P1. Add mochitest. r=gerald
https://hg.mozilla.org/integration/autoland/rev/83dc982bfc63
P2. Fix coding style. r=gerald
https://hg.mozilla.org/integration/autoland/rev/afd77992867f
P3. Fix coding style. r=gerald
https://hg.mozilla.org/integration/autoland/rev/ee6db88f8695
[MSE] P4. Pre-roll AAC by 2112 audio frames. r=gerald

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back these out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=82824092&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/10ad5c6810b3

Comment 16

[Jean-Yves Avenard [:jya]]

Comment on attachment 8845320 [details]
Bug 1345756: P1. Add mochitest.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118502/diff/2-3/

Comment 17

[Jean-Yves Avenard [:jya]]

Comment on attachment 8845321 [details]
Bug 1345756: P2. Fix coding style.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118504/diff/2-3/

Comment 18

[Jean-Yves Avenard [:jya]]

Comment on attachment 8845322 [details]
Bug 1345756: P3. Fix coding style.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118506/diff/2-3/

Comment 19

[Jean-Yves Avenard [:jya]]

Comment on attachment 8845323 [details]
Bug 1345756: [MSE] P4. Pre-roll AAC by 2112 audio frames.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118508/diff/2-3/

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/574f66f6a635
P1. Add mochitest. r=gerald
https://hg.mozilla.org/integration/autoland/rev/9b53eaf9969d
P2. Fix coding style. r=gerald
https://hg.mozilla.org/integration/autoland/rev/abc38feea690
P3. Fix coding style. r=gerald
https://hg.mozilla.org/integration/autoland/rev/e39dc94fc219
[MSE] P4. Pre-roll AAC by 2112 audio frames. r=gerald

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/574f66f6a635
https://hg.mozilla.org/mozilla-central/rev/9b53eaf9969d
https://hg.mozilla.org/mozilla-central/rev/abc38feea690
https://hg.mozilla.org/mozilla-central/rev/e39dc94fc219

Comment 22

[Jean-Yves Avenard [:jya]]

Comment on attachment 8845323 [details]
Bug 1345756: [MSE] P4. Pre-roll AAC by 2112 audio frames.

Approval Request Comment
[Feature/Bug causing the regression]: Been like that for a while
[User impact if declined]: Some Seeks would never complete (including on Youtube). Also this is required for the uplift performed in bug 1342913 to succeed.
[Is this code covered by automated tests?]: yes, new tests were written too.
[Has the fix been verified in Nightly?]: Yes, new mochitests ensure it's working
[Needs manual test from QE? If yes, steps to reproduce]: Can do, https://people-mozilla.org/~jyavenard/tests/mse_mp4/1347101/t.html, if the fix is in, the text "ok(true) seek completed" will appear, otherwise it won't. Can be checked on mac os 10.10.1 (same version as treeherder)
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no. 
[Why is the change risky/not risky?]: We have the same approach with Opus already, just extending to AAC decoder.
[String changes made/needed]: None

Comment 23

[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 24

[Jean-Yves Avenard [:jya]]

A manual verification is going to be very difficult without access to the machine of treeherder.
That's what the mochitest does on treeherder.

If you revert P4, then the test added with P1 will now fail on the OS X runs.


As seen on this run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3929c97fb4863e98e9793de8d15e429d4e3a957

"TEST-UNEXPECTED-FAIL | dom/media/mediasource/test/test_SeekToLastFrame_mp4.html | Test timed out. "

Comment 25

[Gerry Chang [:gchang]]

Comment on attachment 8845323 [details]
Bug 1345756: [MSE] P4. Pre-roll AAC by 2112 audio frames.

Fix an video playback issue on OS X treeherder. Aurora54+ & Beta53+.

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e5d344d97791
https://hg.mozilla.org/releases/mozilla-aurora/rev/8419d3c2e100

Comment 27

[Carsten Book [:Tomcat]]

needs beta rebasing

grafting 407219:e5d344d97791 "Bug 1345756: [MSE] P4. Pre-roll AAC by 2112 audio frames. r=gerald a=gchang"
merging dom/media/mediasource/MediaSourceDemuxer.cpp
warning: conflicts while merging dom/media/mediasource/MediaSourceDemuxer.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')

Comment 28

[Brindusa Tot, Desktop QA]

I reproduced this issue on Nightly 55.0a1 from 03/12/2017, Build ID 20170312030213 on Mac OS X 10.10. I tested with video from https://people-mozilla.org/~jyavenard/tests/mse_mp4/1347101/t.html - video does not start playing and the text "ok(true) seek completed" does NOT appear.

I can confirm the fix on latest Nightly 55.0a1, Build ID 20170326030204 on Mac OS X 10.10. I tested with same video https://people-mozilla.org/~jyavenard/tests/mse_mp4/1347101/t.html - video start playing and the text "ok(true) seek completed" is displayed.

Comment 29

[Jean-Yves Avenard [:jya]]

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/ca8690c486b3682b0b57294b3a9890ef07d9c973
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/c07fb91ad209186b9644e1e65fc4bc4f8996240b

Comment 30

[Mihai Boldan, Desktop QA [:mboldan]]

Managed to reproduce the issue on Firefox 53.0b7, under Mac OS X 10.11.
The issue is no longer reproducible on Firefox 53.0b8, or on Firefox 54.0a2 (2017-03-31).
Tests were performed under Mac OS X 10.10.4 and under Mac OS X 10.11.