This was causing the tests in bug 1063323 to time out intermittently. See bug 1091008 comment 32, which I decided to split out into a separate bug. Patch coming up.

from bug 1091008 comment 32: > So the MSE stuff is racey here. I've compared TBPL NSPR logs between a good > and bad run, and here's what's happening: > > (1) We append data to the source buffer. It gets piped down into the track > buffer, which schedules the state machine thread. It also queues up decoder > initialization to happen asynchronously on the decode thread. > (2A) The state machine thread wakes up, and requests data from the reader. > (2B) The decoder gets initialized and added to mInitializedDecoders. > (3) The reader tries to process the request for video data. If (2A) happens > faster than (2B), it doesn't find anything in mInitializedDecoders with the > appropriate range, and fires OnNotDecoded(WAITING_FOR_DATA), which causes > the state machine to hang. > > One easy fix here is to put the state machine into buffering mode when we > receiving OnNotDecoded(WAITING_FOR_DATA) - this will ensure that it tries > again, and generally feels like the correct thing to do. > > We could also have the reader do additional checks to see if there are any > not-yet-initialized decoders, and wait to respond to the state machine until > that's sorted out. That generally feels a lot less robust, though. > > I'll attach a patch.

Comment on attachment 8520209 [details] [diff] [review] Switch to buffering mode on WAITING_FOR_DATA. v1 Review of attachment 8520209 [details] [diff] [review]: ----------------------------------------------------------------- Does it make sense to also have a callback on the RequestSampleCallback interface to say "I'm not longer waiting for data", to move us out of the BUFFERING state? Then rather than polling once per second in the BUFFERING case handler in MediaDecoderStateMachine to determine whether we should move out of buffering state, we could move out of buffering state as soon we're able to. This would improve the latency. If you think this makes sense, we should do it in another patch.

Ideally update and updateend events and changes to buffered should not happen until after ReadMetadata has run to initialize the new decoder. Ignore comment 5.

Still tinkering with this: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=98c5bde79d39 (orange, made some fixes) https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6287c12a8999 (In reply to Karl Tomlinson (:karlt) from comment #6) > Ideally update and updateend events and changes to buffered should not happen > until after ReadMetadata has run to initialize the new decoder. Yeah - do you think we should get a bug on file for that?

(In reply to Bobby Holley (:bholley) from comment #7) > (In reply to Karl Tomlinson (:karlt) from comment #6) > > Ideally update and updateend events and changes to buffered should not happen > > until after ReadMetadata has run to initialize the new decoder. > > Yeah - do you think we should get a bug on file for that? I don't know what priority we should give it, but I filed bug 1097252.

This wasn't quite right - we only want to switch to buffering mode when we're out of samples.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d057f1ed36eb https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=dc17724b71ef