This is the last big step towards eliminating the decoder monitor. I have patches for the NotifyDataArrived stuff that are mostly green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=adc9d9b13738 I have the .buffered stuff working for non-MSE, but I've run into some difficulties with MSE. Jean-Yves has been deep in that code for a while, so he's going to take a look.

With this change, we don't need to condition this work on infinite streams.

NotifyDataArrived will soon run off-main-thread, so the assumptions here won't hold.

It would be nice to remove the argument in a separate patch, but we can't perform MediaResource reads on the main thread, so the SilentReadAt stuff needs to happen at the same time as the off-main-thread stuff.

This currently asserts for the MSE case.

NI jya to take a look at completing part 6. Note that you'll need to make sure you have bug 1172264 applied (landed on inbound 8 hours ago, but hasn't merged to m-c yet). Also note that the patches through Part 4 have a couple of failures on try [1], so don't worry if you run into those. Just get something more or less working if you can and I'll polish everything up tomorrow. [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=adc9d9b13738

Comment on attachment 8623992 [details] [diff] [review] Part 5 - Misc changes to Intervals/TimeUnit. v1 Review of attachment 8623992 [details] [diff] [review]: ----------------------------------------------------------------- I wasn't asked for review, but somehow I feel I should have been :)

Quick Note: MediaDecoderReader::NotifyDataRemoved() must be handled the same way as NotifyDataArrived -> not running on the main thread. We can't just remove the use of the main thread demuxer and stop notifying it that new data has been added or removed; as it's required for calculating the eviction offset which also is called on the main thread. Only once those are also implemented in that fashion (maybe simply cache the eviction offset value when NotifyDataArrived / NotifyDataRemoved has been called) would we be able to remove the main thread demuxer. Maybe time to remove MP4Reader while at it, because it's going to get in the way.

I'm not sure where to put this or if this is related to WIP on this patch. I've hit several crashes at various places. One is MediaFormatReader::Shutdown() being called after MediaDecoderReader::mDecoder was set to null. This causes a crash in MediaFormatReader::ReleaseMediaResource (but so would MP4Reader) backtrace: (lldb) print mDecoder (mozilla::AbstractMediaDecoder *) $0 = 0x0000000000000000 (lldb) bt * thread #100: tid = 0x72005c, 0x00000001065c3b67 XUL`mozilla::MediaFormatReader::ReleaseMediaResources(this=0x000000012d8a0000) + 23 at MediaFormatReader.cpp:1345, name = 'MediaPl~back #3', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x00000001065c3b67 XUL`mozilla::MediaFormatReader::ReleaseMediaResources(this=0x000000012d8a0000) + 23 at MediaFormatReader.cpp:1345 frame #1: 0x00000001065a6c3c XUL`mozilla::MediaDecoderReader::Shutdown(this=0x000000012d8a0000) + 188 at MediaDecoderReader.cpp:364 frame #2: 0x00000001065bc542 XUL`mozilla::MediaFormatReader::Shutdown(this=0x000000012d8a0000) + 1250 at MediaFormatReader.cpp:146 frame #3: 0x00000001067859fa XUL`mozilla::DelayedDispatchToMainThread::Run(this=0x00000001222070d0) + 58 at TrackBuffer.cpp:1150 frame #4: 0x0000000106587b69 XUL`mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run(this=0x00000001355de120) + 281 at TaskDispatcher.h:181 frame #5: 0x000000010669f62d XUL`mozilla::MediaTaskQueue::Runner::Run(this=0x000000012d25aa20) + 605 at MediaTaskQueue.cpp:256 frame #6: 0x000000010371fa08 XUL`nsThreadPool::Run(this=0x00000001467448a0) + 984 at nsThreadPool.cpp:221 frame #7: 0x000000010371fb0c XUL`non-virtual thunk to nsThreadPool::Run(this=0x00000001467448a8) + 28 at nsThreadPool.cpp:235 frame #8: 0x000000010371c504 XUL`nsThread::ProcessNextEvent(this=0x0000000145ea13e0, aMayWait=false, aResult=0x000000012d501c5e) + 2068 at nsThread.cpp:848 frame #9: 0x00000001037929f7 XUL`NS_ProcessNextEvent(aThread=0x0000000145ea13e0, aMayWait=false) + 151 at nsThreadUtils.cpp:265 frame #10: 0x0000000103dc9e20 XUL`mozilla::ipc::MessagePumpForNonMainThreads::Run(this=0x00000001465d9680, aDelegate=0x00000001460ef920) + 656 at MessagePump.cpp:326 frame #11: 0x0000000103d372a5 XUL`MessageLoop::RunInternal(this=0x00000001460ef920) + 117 at message_loop.cc:234 frame #12: 0x0000000103d371b5 XUL`MessageLoop::RunHandler(this=0x00000001460ef920) + 21 at message_loop.cc:227 frame #13: 0x0000000103d3715d XUL`MessageLoop::Run(this=0x00000001460ef920) + 45 at message_loop.cc:201 frame #14: 0x000000010371aa29 XUL`nsThread::ThreadFunc(aArg=0x0000000145ea13e0) + 329 at nsThread.cpp:360 frame #15: 0x00000001033715d1 libnss3.dylib`_pt_root(arg=0x0000000146020410) + 449 at ptthread.c:212 frame #16: 0x00007fff9384a268 libsystem_pthread.dylib`_pthread_body + 131 frame #17: 0x00007fff9384a1e5 libsystem_pthread.dylib`_pthread_start + 176 frame #18: 0x00007fff9384841d libsystem_pthread.dylib`thread_start + 13 (lldb) One encountered a lot is: the assertion is: MOZ_DIAGNOSTIC_ASSERT(aFailureHandling == DontAssertDispatchSuccess || NS_SUCCEEDED(rv)); * thread #1: tid = 0x641d60, 0x0000000106681dcd XUL`mozilla::MediaTaskQueue::Dispatch(this=0x000000012ab09010, aRunnable=(mRawPtr = 0x0000000000000000), aFailureHandling=AssertDispatchSuccess, aReason=TailDispatch) + 237 at MediaTaskQueue.h:53, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x0000000106681dcd XUL`mozilla::MediaTaskQueue::Dispatch(this=0x000000012ab09010, aRunnable=(mRawPtr = 0x0000000000000000), aFailureHandling=AssertDispatchSuccess, aReason=TailDispatch) + 237 at MediaTaskQueue.h:53 frame #1: 0x0000000106586a18 XUL`mozilla::AutoTaskDispatcher::DispatchTaskGroup(this=0x0000000117e4d188, aGroup=UniquePtr<mozilla::AutoTaskDispatcher::PerThreadTaskGroup, mozilla::DefaultDelete<mozilla::AutoTaskDispatcher::PerThreadTaskGroup> > at 0x00007fff5fbfcbb8) + 200 at TaskDispatcher.h:233 frame #2: 0x0000000106588098 XUL`mozilla::AutoTaskDispatcher::~AutoTaskDispatcher(this=0x0000000117e4d188) + 232 at TaskDispatcher.h:88 frame #3: 0x0000000106586115 XUL`mozilla::AutoTaskDispatcher::~AutoTaskDispatcher(this=0x0000000117e4d188) + 21 at TaskDispatcher.h:75 frame #4: 0x000000010658568f XUL`mozilla::Maybe<mozilla::AutoTaskDispatcher>::reset(this=0x0000000117e4d180) + 63 at Maybe.h:373 frame #5: 0x0000000106585981 XUL`mozilla::XPCOMThreadWrapper::FireTailDispatcher(this=0x0000000117e4d160) + 161 at AbstractThread.cpp:76 frame #6: 0x0000000106585eb3 XUL`void nsRunnableMethodArguments<>::apply<mozilla::XPCOMThreadWrapper, void (this=0x000000012b77cff0, o=0x0000000117e4d160, m=0x00000001065858e0)()>(mozilla::XPCOMThreadWrapper*, void (mozilla::XPCOMThreadWrapper::*)()) + 131 at nsThreadUtils.h:618 frame #7: 0x0000000106585b4c XUL`nsRunnableMethodImpl<void (mozilla::XPCOMThreadWrapper::*)(), true>::Run(this=0x000000012b77cfc0) + 140 at nsThreadUtils.h:825 frame #8: 0x00000001070670fe XUL`nsBaseAppShell::RunSyncSectionsInternal(this=0x000000011f277660, aStable=true, aThreadRecursionLevel=0) + 462 at nsBaseAppShell.cpp:376 frame #9: 0x000000010707a31d XUL`nsBaseAppShell::RunSyncSections(this=0x000000011f277660, stable=true, threadRecursionLevel=0) + 77 at nsBaseAppShell.h:95 frame #10: 0x00000001070673bd XUL`nsBaseAppShell::AfterProcessNextEvent(this=0x000000011f277660, thr=0x0000000100431310, recursionDepth=0, eventWasProcessed=true) + 45 at nsBaseAppShell.cpp:435 frame #11: 0x00000001070dc893 XUL`nsAppShell::AfterProcessNextEvent(this=0x000000011f277660, aThread=0x0000000100431310, aRecursionDepth=0, aEventWasProcessed=true) + 307 at nsAppShell.mm:762 frame #12: 0x00000001070dc944 XUL`non-virtual thunk to nsAppShell::AfterProcessNextEvent(this=0x000000011f277668, aThread=0x0000000100431310, aRecursionDepth=0, aEventWasProcessed=true) + 68 at nsAppShell.mm:766 frame #13: 0x000000010371be24 XUL`nsThread::ProcessNextEvent(this=0x0000000100431310, aMayWait=false, aResult=0x00007fff5fbfd063) + 2596 at nsThread.cpp:862 frame #14: 0x0000000103791f67 XUL`NS_ProcessPendingEvents(aThread=0x0000000100431310, aTimeout=20) + 151 at nsThreadUtils.cpp:207 frame #15: 0x0000000107066656 XUL`nsBaseAppShell::NativeEventCallback(this=0x000000011f277660) + 198 at nsBaseAppShell.cpp:99 frame #16: 0x00000001070db68d XUL`nsAppShell::ProcessGeckoEvents(aInfo=0x000000011f277660) + 445 at nsAppShell.mm:378 frame #17: 0x00007fff8f425a01 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 frame #18: 0x00007fff8f417b8d CoreFoundation`__CFRunLoopDoSources0 + 269 frame #19: 0x00007fff8f4171bf CoreFoundation`__CFRunLoopRun + 927 frame #20: 0x00007fff8f416bd8 CoreFoundation`CFRunLoopRunSpecific + 296 frame #21: 0x00007fff8f8fb56f HIToolbox`RunCurrentEventLoopInMode + 235 frame #22: 0x00007fff8f8fb2ea HIToolbox`ReceiveNextEventCommon + 431 frame #23: 0x00007fff8f8fb12b HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 71 frame #24: 0x00007fff8e5d59bb AppKit`_DPSNextEvent + 978 frame #25: 0x00007fff8e5d4f68 AppKit`-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 346 frame #26: 0x00000001070da1b7 XUL`-[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:](self=0x0000000117e4d4c0, _cmd=0x00007fff8eef1808, mask=18446744073709551615, expiration=0x422d63c37f00000d, mode=0x00007fff7d444040, flag='\x01') + 119 at nsAppShell.mm:119 frame #27: 0x00007fff8e5cabf3 AppKit`-[NSApplication run] + 594 frame #28: 0x00000001070dc061 XUL`nsAppShell::Run(this=0x000000011f277660) + 177 at nsAppShell.mm:653 frame #29: 0x00000001080fb831 XUL`nsAppStartup::Run(this=0x000000011f2790b0) + 161 at nsAppStartup.cpp:280 frame #30: 0x00000001081c54cb XUL`XREMain::XRE_mainRun(this=0x00007fff5fbfefe0) + 5963 at nsAppRunner.cpp:4263 frame #31: 0x00000001081c5ded XUL`XREMain::XRE_main(this=0x00007fff5fbfefe0, argc=5, argv=0x00007fff5fbff8e0, aAppData=0x00007fff5fbff268) + 989 at nsAppRunner.cpp:4347 frame #32: 0x00000001081c6292 XUL`XRE_main(argc=5, argv=0x00007fff5fbff8e0, aAppData=0x00007fff5fbff268, aFlags=0) + 98 at nsAppRunner.cpp:4436 frame #33: 0x00000001000029fb firefox`do_main(argc=5, argv=0x00007fff5fbff8e0, xreDirectory=0x000000010040df80) + 1819 at nsBrowserApp.cpp:214 frame #34: 0x0000000100001dcf firefox`main(argc=5, argv=0x00007fff5fbff8e0) + 287 at nsBrowserApp.cpp:478 frame #35: 0x0000000100001854 firefox`start + 52 this is mostly seen on the youtube MSE conformance test. Dispatch failure, would that mean that the target task queue has been shutdown while there are still tail dispatch task pending ???

The main thread demuxer still needs to be retained at this stage, in order to properly calculate GetEvictionOffset() this could also be reworked to work on the reader's thread.

Only resolve AppendPromise once we know all our sub-readers have recalculated their buffered ranges.

Handle sourcebuffer eviction. We can't call GetBuffered on the main thread. So we cache the buffered ranges within the track buffer.

All of the old MSE is handled I believe. about:media plugin is broken with this as it calls GetBuffered() on the main thread to build its string. If the buffered range of all sub-readers were mirrored, we could read it that way ; or do the same dirty trick as I did in patch P6-4 and query all trackbuffers until we found the buffered ranges for the required decoder... Or better, get rid of this MSE/MediaSourceReader and use the new one :)

Thanks for helping move this forward! (In reply to Jean-Yves Avenard [:jya] from comment #10) > I'm not sure where to put this or if this is related to WIP on this patch. Yeah, let's keep them separate for now, and then roll all of the part 6 pieces together before landing. > I've hit several crashes at various places. > One is MediaFormatReader::Shutdown() being called after > MediaDecoderReader::mDecoder was set to null. This is puzzling to me (and I can't reproduce it). In your stack, The call to ReleaseMediaResources is coming from MediaDecoderReader::Shutdown, and occurs _before_ the part where we set mDecoder = nullptr. Does this still reproduce? > One encountered a lot is: > the assertion is: > MOZ_DIAGNOSTIC_ASSERT(aFailureHandling == DontAssertDispatchSuccess || > NS_SUCCEEDED(rv)); Fixed this issue in the updated patches I'm about to upload - DispatchNotifyDataArrived shouldn't assert dispatch success.

I now get the following crash on the YouTube suite: Assertion failure: !Exists(), at /files/mozilla/repos/dd/dom/media/MediaPromise.h:818 #01: mozilla::MediaPromiseRequestHolder<mozilla::MediaPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true> >::Begin(mozilla::MediaPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true>::Request*)[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3062fc5] #02: mozilla::TrackBuffer::InitializeDecoder(mozilla::SourceBufferDecoder*)[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x31cf50f] #03: void nsRunnableMethodArguments<mozilla::SourceBufferDecoder*>::apply<mozilla::TrackBuffer, void (mozilla::TrackBuffer::*)(mozilla::SourceBufferDecoder*)>(mozilla::TrackBuffer*, void (mozilla::TrackBuffer::*)(mozilla::SourceBufferDecoder*))[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x31f17cf] #04: nsRunnableMethodImpl<void (mozilla::TrackBuffer::*)(mozilla::SourceBufferDecoder*), true, mozilla::SourceBufferDecoder*>::Run()[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x31f14fc] #05: mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run()[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3002ca9] #06: mozilla::MediaTaskQueue::Runner::Run()[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x311a8fd] #07: nsThreadPool::Run()[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x19a998] #08: non-virtual thunk to nsThreadPool::Run()[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x19aa9c] #09: nsThread::ProcessNextEvent(bool, bool*)[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x197494] #10: NS_ProcessNextEvent(nsIThread*, bool)[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x20d9d7] #11: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x84510e] #12: MessageLoop::RunInternal()[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7b2485] #13: MessageLoop::RunHandler()[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7b2395] #14: MessageLoop::Run()[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7b233d] #15: nsThread::ThreadFunc(void*)[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1959b9] #16: _pt_root[/files/mozilla/repos/dd/obj-x86_64-apple-darwin14.3.0/dist/NightlyDebug.app/Contents/MacOS/libnss3.dylib +0x3718d1] #17: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x3268] #18: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x31e5] Process 29802 stopped * thread #136: tid = 0x951993, 0x00000001045e7fc9 XUL`mozilla::MediaPromiseRequestHolder<mozilla::MediaPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true> >::Begin(this=0x000000011e227b10, aRequest=0x0000000127fa2160) + 89 at MediaPromise.h:818, name = 'MediaPl~back #1', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) frame #0: 0x00000001045e7fc9 XUL`mozilla::MediaPromiseRequestHolder<mozilla::MediaPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true> >::Begin(this=0x000000011e227b10, aRequest=0x0000000127fa2160) + 89 at MediaPromise.h:818 815 816 void Begin(typename PromiseType::Request* aRequest) 817 { -> 818 MOZ_DIAGNOSTIC_ASSERT(!Exists()); 819 mRequest = aRequest; 820 } 821 This is deep in TrackBuffer, so I'll leave investigation to you. I'm also getting one failure in our MSE test suite: dom/media/mediasource/test/test_TruncatedDuration.html | sourcebuffer truncated - got 4.001, expected 1.334

This probably isn't the right thing, but I needed this to avoid null derefs.

Let's see what else try says: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3b2302c2480

Comment on attachment 8624494 [details] [diff] [review] Misc Fixes. v2 Review of attachment 8624494 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/mediasource/TrackBuffer.cpp @@ +545,5 @@ > } > > if (evicted) { > ProxyMediaCall(mParentDecoder->GetReader()->TaskQueue(), this, __func__, > + &TrackBuffer::UpdateBufferedRanges, uint32_t(0), int64_t(0)); don't you like the original K&R C ??

This fixes the test_playback_rate failure.

This includes an 'explicit' to make the static analysis happy.

This should hopefully fix the b2g failures.

Only resolve AppendPromise once we know all our sub-readers have recalculated their buffered ranges.

Handle sourcebuffer eviction. We can't call GetBuffered on the main thread. So we cache the buffered ranges within the track buffer.

Comment on attachment 8624569 [details] [diff] [review] P6-5. Misc Fix #2 folded across P6-3 and P6-4

Comment on attachment 8623988 [details] [diff] [review] Part 1 - Make the logic in MDSM::NotifyDataArrived apply to mObservedDuration. v1 Review of attachment 8623988 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/MediaDecoderStateMachine.cpp @@ -1646,5 @@ > - if (!mDecoder->IsInfinite() || !HaveStartTime()) > - { > - return; > - } > - Removing this will cause bug 1166836 to come back. I too thought it could be removed and it bit me big time. NotifyDataArrived() is called every 32kB (or maybe 64 can't remember) the MediaResource download data. With a long MP4 this causes the process to recalculate the getbuffered range. I just tried the video in that bug and indeed, seeing 180%+ CPU usage, and NotifyDemuxer being called thousand of times in a row. Now the situation is better because it's not done on the main thread anymore, but it's still unnecessary. This particular code is just a dirty hack IMHO, and only there to ensure that we have at least once the buffered range calculated. So we should only run this code once and if it has run before, skip it.

Comment on attachment 8623989 [details] [diff] [review] Part 2 - Dispatch UpdateEstimatedMediaDuration. v1 Review of attachment 8623989 [details] [diff] [review]: ----------------------------------------------------------------- \o/

Comment on attachment 8623989 [details] [diff] [review] Part 2 - Dispatch UpdateEstimatedMediaDuration. v1 Review of attachment 8623989 [details] [diff] [review]: ----------------------------------------------------------------- Of course \o/ meant r+

Comment on attachment 8624560 [details] [diff] [review] Part 6-1 - Use mirroring for buffered ranges. P6-1 v2 Review of attachment 8624560 [details] [diff] [review]: ----------------------------------------------------------------- I'll wait for all 6 to be reviewed and at least P6-2 other you have a regression in how GetEvictionOffset will return invalid data. how it is currently done: it can't work

Comment on attachment 8624565 [details] [diff] [review] Part 3 - Implement SilentReadAt. v2 Review of attachment 8624565 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/MediaResource.h @@ +292,5 @@ > + > + // ReadAt without side-effects. Given that our MediaResource infrastructure > + // is very side-effecty, this accomplishes its job by checking the initial > + // position and seeking back to it. If the seek were to fail, a side-effect > + // might be observable. it seems rather pointless to seek back; especially if you have two thread attempt to read the mediaresource at the same time; there's no guarantee anyway that Tell() will return anything meaningfull. AFAIK, only the WebM reader does that, and this is why we can't use it with MSE as it's not thread-safe. @@ +298,5 @@ > + // This method returns null if anything fails, including the failure to read > + // aCount bytes. Otherwise, it returns an owned buffer. > + virtual mozilla::UniquePtr<char[]> SilentReadAt(int64_t aOffset, uint32_t aCount) > + { > + mozilla::UniquePtr<char[]> bytes(new (fallible) char[aCount]); why not return an already_addRefed<MediaByteBuffer> (and if you want to ensure there's ownership transfer return a nsRefPtr<MediaByteBuffer> call SetCapacity with fallible as 2nd argument

The main thread demuxer still needs to be retained at this stage, in order to properly calculate GetEvictionOffset() this could also be reworked to work on the reader's thread.

Comment on attachment 8624583 [details] [diff] [review] . P6-2 Remove now unused members. actually no, it won't help...

Comment on attachment 8623988 [details] [diff] [review] Part 1 - Make the logic in MDSM::NotifyDataArrived apply to mObservedDuration. v1 Per IRC discussion, this method actually goes away entirely by the end of the patch stack. What we need is potentially a way to throttle NotifyDataArrived notifications for the reader, which I'll look into tomorrow.

Comment on attachment 8623988 [details] [diff] [review] Part 1 - Make the logic in MDSM::NotifyDataArrived apply to mObservedDuration. v1 Review of attachment 8623988 [details] [diff] [review]: ----------------------------------------------------------------- several possibilities to reduce CPU usage. In MediaFormatReader, only schedule an update is mEOS or mWaitingForData is true. Buffer the NotifyDataReceived in the same fashion I did (Code that was removed in part6). That is keep a TimeInterval that is updated with the span of each call to NotifyDataArrived and schedule an update if EOS or Waiting for data or if last NotifyDataArrived was more than say 1s. Have the MDSM perform a similar buffering.. We don't need to know that there's new data that is coming that often...

(In reply to Jean-Yves Avenard [:jya] from comment #37) > it seems rather pointless to seek back; especially if you have two thread > attempt to read the mediaresource at the same time; there's no guarantee > anyway that Tell() will return anything meaningfull. In principal I agree with you - the API should only provide Read, not ReadAt. But I want to minimize the risk of weird regressions, so I'm just doing it for now. We can fix it when we remove Read() and Tell() from MediaResource. > why not return an already_addRefed<MediaByteBuffer> (and if you want to > ensure there's ownership transfer return a nsRefPtr<MediaByteBuffer> > call SetCapacity with fallible as 2nd argument Done - turned out to be a lot of work though, especially because of the type mismatch between char* and unsigned char*.

So, this seems to be more orange than before on the mediasource mochitests and webref: Last night's push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5135e30a068a Today's push, with a fix for the webm crash: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cadc97732eec Attaching my complete current patch stack, rebased against trunk. Jean-Yves, can you take a look at the MSE failures?

(In reply to Jean-Yves Avenard [:jya] from comment #41) > Comment on attachment 8623988 [details] [diff] [review] > Part 1 - Make the logic in MDSM::NotifyDataArrived apply to > mObservedDuration. v1 > > Review of attachment 8623988 [details] [diff] [review]: > ----------------------------------------------------------------- > > several possibilities to reduce CPU usage. > > In MediaFormatReader, only schedule an update is mEOS or mWaitingForData is > true. > > Buffer the NotifyDataReceived in the same fashion I did (Code that was > removed in part6). That is keep a TimeInterval that is updated with the span > of each call to NotifyDataArrived and schedule an update if EOS or Waiting > for data or if last NotifyDataArrived was more than say 1s. I'm confused - you're suggesting that the primary overhead is in NotifyDemuxer and the associated Update? That overhead exists before the patch as well. It seems to me that the problem is the UpdateBuffered call in MediaDecoderReader::NotifyDataArrived. I'll write up a patch to throttle that.

(In reply to Bobby Holley (:bholley) from comment #44) > I'm confused - you're suggesting that the primary overhead is in > NotifyDemuxer and the associated Update? That overhead exists before the > patch as well. It seems to me that the problem is the UpdateBuffered call in > MediaDecoderReader::NotifyDataArrived. I'll write up a patch to throttle > that. Oh, it looks like the NotifyDataArrived implementation in MediaFormatReader also invokes GetBuffered. So I'll throttle both in my patch.

This reduces the time spent in GetBuffered on the channel9 video from ~40% to ~1.5% on my machine.

(In reply to Bobby Holley (:bholley) from comment #43) > Today's push, with a fix for the webm crash: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=cadc97732eec Looks like the remaining failures are MSE-related, plus some omx stuff. I've tweaked the OMX stuff, and added the throttling. Hopefully with this push we'll be down to just the MSE failures. https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bd05cbf006c

Comment on attachment 8625008 [details] [diff] [review] Throttle NotifyDataArrived. v1 Review of attachment 8625008 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/MediaDecoderReader.h @@ +258,5 @@ > + return *this; > + } > + > + uint32_t mLength; > + int64_t mOffset; all of this could be more elegantly done with an Interval<int64_t> (or MediaByteRange) Check how I did it in MediaFormatReader before patch 6. All ranges provided in NotifyDataArrived are incrementals so we can simply span one interval after another. ::: dom/media/mediasource/TrackBuffer.cpp @@ +318,5 @@ > // XXX: Params make no sense to parent decoder as it relates to a > // specific SourceBufferDecoder's data stream. Pass bogus values here to > // force parent decoder's state machine to recompute end time for > // infinite length media. > + parent->NotifyDataArrived(0, 0, /* aThrottleUpdates = */ false); we actually can get rid of that one as it's done in SourceBuffer now

(In reply to Bobby Holley (:bholley) from comment #45) > (In reply to Bobby Holley (:bholley) from comment #44) > > I'm confused - you're suggesting that the primary overhead is in > > NotifyDemuxer and the associated Update? That overhead exists before the > > patch as well. It seems to me that the problem is the UpdateBuffered call in > > MediaDecoderReader::NotifyDataArrived. I'll write up a patch to throttle > > that. > > Oh, it looks like the NotifyDataArrived implementation in MediaFormatReader > also invokes GetBuffered. So I'll throttle both in my patch. This used to be implicitly throttled by ScheduleUpdate as it would only schedule an update if none were pending. This significantly reduced the load compare to how the MP4Reader was doing it. We only need to perform the GetBuffered if we are in EOS or WAITING_FOR_DATA, can look into implementing this in another patch. But I think the throttling based on the last time is sufficient enough.

(In reply to Bobby Holley (:bholley) from comment #47) > (In reply to Bobby Holley (:bholley) from comment #43) > > Today's push, with a fix for the webm crash: > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=cadc97732eec > > Looks like the remaining failures are MSE-related, plus some omx stuff. I've > tweaked the OMX stuff, and added the throttling. Hopefully with this push > we'll be down to just the MSE failures. > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bd05cbf006c We have the assertions about the mOffset != aOffset This could be removed by using Interval<int64_t>and use Interval.Span()

(In reply to Jean-Yves Avenard [:jya] from comment #50) > (In reply to Bobby Holley (:bholley) from comment #47) > > (In reply to Bobby Holley (:bholley) from comment #43) > > > Today's push, with a fix for the webm crash: > > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=cadc97732eec > > > > Looks like the remaining failures are MSE-related, plus some omx stuff. I've > > tweaked the OMX stuff, and added the throttling. Hopefully with this push > > we'll be down to just the MSE failures. > > > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bd05cbf006c > > We have the assertions about the mOffset != aOffset > > This could be removed by using Interval<int64_t>and use Interval.Span() Sure, I'll fix that. Still need to fix the MSE issues though - easy to reproduce by just not applying the throttling patch.

(In reply to Jean-Yves Avenard [:jya] from comment #48) > all of this could be more elegantly done with an Interval<int64_t> (or > MediaByteRange) Fixed to use Interval<int64_t>. > we actually can get rid of that one as it's done in SourceBuffer now Let's save that for a followup.

With a fixed up throttling patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec6018b5bc7c

(In reply to Bobby Holley (:bholley) from comment #54) > (In reply to Jean-Yves Avenard [:jya] from comment #48) > > all of this could be more elegantly done with an Interval<int64_t> (or > > MediaByteRange) > > Fixed to use Interval<int64_t>. > > > we actually can get rid of that one as it's done in SourceBuffer now > > Let's save that for a followup. But then you don't get contiguous intervals passed to NotifyDataArrived (always 0,0)

(In reply to Jean-Yves Avenard [:jya] from comment #57) > But then you don't get contiguous intervals passed to NotifyDataArrived > (always 0,0) That line passes aThrottle = false, so it's unaffected by that codepath. (In reply to Bobby Holley (:bholley) from comment #56) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec6018b5bc7c This is now green everywhere modulo the MSE failures. Over to jya for the last piece.

Fully updated patches, including the throttling patch.

Comment on attachment 8625020 [details] [diff] [review] Throttle NotifyDataArrived. r=jya Review of attachment 8625020 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/MediaDecoderReader.cpp @@ +185,5 @@ > + if (mThrottledInterval.isNothing()) { > + mThrottledInterval.emplace(aInterval); > + } else if (!mThrottledInterval.ref().Contiguous(aInterval)) { > + DoThrottledNotify(); > + mThrottledInterval.emplace(aInterval); if you have any pending mThrottledInterface you could lose it no? shouldn't you dispatch the earlier one (if any) and then schedule this one?

Only resolve AppendPromise once we know all our sub-readers have recalculated their buffered ranges.

Handle sourcebuffer eviction. We can't call GetBuffered on the main thread. So we cache the buffered ranges within the track buffer.

Missed one: (lldb) bt * thread #1: tid = 0x13e697, 0x000000010675843c XUL`mozilla::MediaSourceReader::GetBuffered(this=0x0000000130311800) + 140 at MediaSourceReader.cpp:1013, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) frame #0: 0x000000010675843c XUL`mozilla::MediaSourceReader::GetBuffered(this=0x0000000130311800) + 140 at MediaSourceReader.cpp:1013 frame #1: 0x000000010674ce6f XUL`mozilla::MediaSourceDecoder::GetSeekable(this=0x0000000125382640) + 367 at MediaSourceDecoder.cpp:90 * frame #2: 0x0000000106448a5e XUL`mozilla::dom::HTMLMediaElement::Seek(this=0x0000000135e43800, aTime=4884.0720000000001, aSeekType=Accurate, aRv=0x00007fff5fbf49e8) + 942 at HTMLMediaElement.cpp:1491 frame #3: 0x0000000106448fbe XUL`mozilla::dom::HTMLMediaElement::SetCurrentTime(this=0x0000000135e43800, aCurrentTime=4884.0720000000001, aRv=0x00007fff5fbf49e8) + 46 at HTMLMediaElement.cpp:1401 frame #4: 0x0000000105f9259a XUL`mozilla::dom::HTMLMediaElementBinding::set_currentTime(cx=0x00000001303de850, obj=Handle<JSObject *> at 0x00007fff5fbf4a30, self=0x0000000135e43800, args=JSJitSetterCallArgs at 0x00007fff5fbf4a28) + 186 at HTMLMediaElementBinding.cpp:480 frame #5: 0x00000001060d4a82 XUL`mozilla::dom::GenericBindingSetter(cx=0x00000001303de850, argc=1, vp=0x00007fff5fbf5598) + 706 at BindingUtils.cpp:2526 frame #6: 0x00000001094e75c8 XUL`js::CallJSNative(cx=0x00000001303de850, native=0x00000001060d47c0, args=0x00007fff5fbf5430)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 184 at jscntxtinlines.h:235 frame #7: 0x0000000109475c6e XUL`js::Invoke(cx=0x00000001303de850, args=CallArgs at 0x00007fff5fbf5430, construct=NO_CONSTRUCT) + 1358 at Interpreter.cpp:709 frame #8: 0x000000010945b9e6 XUL`js::Invoke(cx=0x00000001303de850, thisv=0x00007fff5fbf5c78, fval=0x00007fff5fbf5680, argc=1, argv=0x00007fff5fbf5ae0, rval=JS::MutableHandleValue at 0x00007fff5fbf5530) + 902 at Interpreter.cpp:766 frame #9: 0x000000010949da58 XUL`js::InvokeSetter(cx=0x00000001303de850, thisv=0x00007fff5fbf5c78, fval=Value at 0x00007fff5fbf5680, v=JS::HandleValue at 0x00007fff5fbf5678) + 232 at Interpreter.cpp:851 frame #10: 0x000000010958e36a XUL`SetExistingProperty(cx=0x00000001303de850, obj=js::HandleNativeObject at 0x00007fff5fbf5890, id=JS::HandleId at 0x00007fff5fbf5888, v=JS::HandleValue at 0x00007fff5fbf5880, receiver=JS::HandleValue at 0x00007fff5fbf5878, pobj=js::HandleNativeObject at 0x00007fff5fbf5870, shape=js::HandleShape at 0x00007fff5fbf5868, result=0x00007fff5fbf5c50) + 1914 at NativeObject.cpp:2288 frame #11: 0x000000010958d866 XUL`js::NativeSetProperty(cx=0x00000001303de850, obj=js::HandleNativeObject at 0x00007fff5fbf5b20, id=JS::HandleId at 0x00007fff5fbf5b18, value=JS::HandleValue at 0x00007fff5fbf5b10, receiver=JS::HandleValue at 0x00007fff5fbf5b08, qualified=Qualified, result=0x00007fff5fbf5c50) + 838 at NativeObject.cpp:2322 frame #12: 0x000000010954ff73 XUL`js::SetProperty(cx=0x00000001303de850, obj=JS::HandleObject at 0x00007fff5fbf5bc0, id=JS::HandleId at 0x00007fff5fbf5bb8, v=JS::HandleValue at 0x00007fff5fbf5bb0, receiver=JS::HandleValue at 0x00007fff5fbf5ba8, result=0x00007fff5fbf5c50) + 227 at NativeObject.h:1434 frame #13: 0x00000001094b192b XUL`SetPropertyOperation(cx=0x00000001303de850, op=JSOP_SETPROP, lval=JS::HandleValue at 0x00007fff5fbf5ce0, id=JS::HandleId at 0x00007fff5fbf5cd8, rval=JS::HandleValue at 0x00007fff5fbf5cd0) + 491 at Interpreter.cpp:315 frame #14: 0x000000010948e6eb XUL`Interpret(cx=0x00000001303de850, state=0x00007fff5fbf8d20) + 47563 at Interpreter.cpp:2767 frame #15: 0x0000000109482c22 XUL`js::RunScript(cx=0x00000001303de850, state=0x00007fff5fbf8d20) + 706 at Interpreter.cpp:653 frame #16: 0x0000000109475dac XUL`js::Invoke(cx=0x00000001303de850, args=CallArgs at 0x00007fff5fbf9520, construct=NO_CONSTRUCT) + 1676 at Interpreter.cpp:729 frame #17: 0x0000000109c0b29a XUL`js::fun_apply(cx=0x00000001303de850, argc=2, vp=0x00007fff5fbfa8f8) + 1706 at jsfun.cpp:1289 frame #18: 0x00000001094e75c8 XUL`js::CallJSNative(cx=0x00000001303de850, native=0x0000000109c0abf0, args=0x00007fff5fbfa790)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 184 at jscntxtinlines.h:235 frame #19: 0x0000000109475c6e XUL`js::Invoke(cx=0x00000001303de850, args=CallArgs at 0x00007fff5fbfa790, construct=NO_CONSTRUCT) + 1358 at Interpreter.cpp:709 frame #20: 0x000000010945b9e6 XUL`js::Invoke(cx=0x00000001303de850, thisv=0x00007fff5fbfab90, fval=0x00007fff5fbfabc0, argc=2, argv=0x00007fff5fbfad28, rval=JS::MutableHandleValue at 0x00007fff5fbfa890) + 902 at Interpreter.cpp:766 frame #21: 0x00000001097f41a9 XUL`js::jit::DoCallFallback(cx=0x00000001303de850, frame=0x00007fff5fbfadc8, stub_=0x000000012742f890, argc=2, vp=0x00007fff5fbfad18, res=JS::MutableHandleValue at 0x00007fff5fbfac78) + 1849 at BaselineIC.cpp:9855 frame #22: 0x000000011769320b When seeking, HTMLMediaElement does media::TimeIntervals seekableIntervals = mDecoder->GetSeekable(); on the main thread, which calls MediaSourceDecoder::GetBuffered(). Everything in there is thread safe provided you hold the decoder's monitor. GetSeekable should use the buffered mirror no? surprising that none of the mochitest/webref triggers it. i got it trying this page: http://steamcommunity.com/broadcast/watch/76561198047709840

Comment on attachment 8624560 [details] [diff] [review] Part 6-1 - Use mirroring for buffered ranges. P6-1 v2 Review of attachment 8624560 [details] [diff] [review]: ----------------------------------------------------------------- In MediaSourceDecoder, changing GetSeekable so it calls GetBuffered rather than mReader->GetBuffered() is required (to prevent assert). i have the feeling that the MSE is still wrong in how it's going to report the mediasource buffered range. if mediasource go into ended mode, it must update the buffered ranges as there are rules on what the buffered attribute is supposed to return when the mediasource is ended (all trackbuffers' end are aligned with the longest) ::: dom/media/MediaFormatReader.cpp @@ +1311,5 @@ > + if (HasAudio()) { > + UpdateReceivedNewData(TrackType::kAudioTrack); > + } > + if (HasVideo()) { > + MonitorAutoLock lock(mVideo.mMonitor); the lock isn't required anymore @@ +1315,5 @@ > + MonitorAutoLock lock(mVideo.mMonitor); > + videoti = mVideo.mTimeRanges; > + } > + if (HasAudio()) { > + MonitorAutoLock lock(mAudio.mMonitor); same with this one. doc in MediaFormatReader.h should be amended, and definition of mTimeRanges moved up.

ah! indeed: TEST-UNEXPECTED-FAIL | /media-source/mediasource-buffered.html | Demuxed content with different lengths - assert_equals: mediaElement.buffered expected "{ [0.000, 2.043) }" but got "{ [0.000, 2.000) }" that's the case I described above... when going into ended mode, we could dispatch a task that update the buffered ranges. however, i believe it's supposed to be synchronous. not sure on how that can be done now. looking at the MSE spec, we are to run the range removal algorithm (which is async), but here we would have nothing to remove ; it's a bit blurry as such if the buffered attributes is to be updated immediately or not.

(In reply to Jean-Yves Avenard [:jya] from comment #60) > ::: dom/media/MediaDecoderReader.cpp > @@ +185,5 @@ > > + if (mThrottledInterval.isNothing()) { > > + mThrottledInterval.emplace(aInterval); > > + } else if (!mThrottledInterval.ref().Contiguous(aInterval)) { > > + DoThrottledNotify(); > > + mThrottledInterval.emplace(aInterval); > > if you have any pending mThrottledInterface you could lose it no? shouldn't > you dispatch the earlier one (if any) and then schedule this one? That's exactly what this code does, unless I'm misunderstanding something. (In reply to Jean-Yves Avenard [:jya] from comment #66) > doc in MediaFormatReader.h should be amended, and definition of mTimeRanges > moved up. By this I assume you mean removing mMonitor entirely, and the one other place where we acquire it (in MediaFormatReader::UpdateReceivedNewData), right? I'll do that, but please let me know if that's wrong for some reason.

(In reply to Jean-Yves Avenard [:jya] from comment #67) > looking at the MSE spec, we are to run the range removal algorithm (which is > async), but here we would have nothing to remove ; it's a bit blurry as such > if the buffered attributes is to be updated immediately or not. Yeah - the spec indicates that we should update the duration, but I don't see anything in the spec that indicates that .buffered should be updated synchronously. Do you?

Comment on attachment 8625161 [details] [diff] [review] Use mirroring for buffered ranges. r=jya Just double-checking that I did the right thing wrt the monitor.

Final all-platform try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a60e71513dc

Rebased patches.

Comment on attachment 8625162 [details] [diff] [review] Mark probably-invalid webref test as failing. v1 Review of attachment 8625162 [details] [diff] [review]: ----------------------------------------------------------------- The test isn't wrong I don't think. Now we may want to ignore it for now. Only data eviction is asynchronous. How we filter the buffered range upon call to endOfStream is another matter. What we need to do is query the source buffer buffered ranges and apply what the spec requires. TrackBuffer can use its own monitor to protect its cached buffered range rather than relying on the MediaSourceDecoder monitor. I'll write something up later

Comment on attachment 8625161 [details] [diff] [review] Use mirroring for buffered ranges. r=jya Review of attachment 8625161 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/MediaFormatReader.cpp @@ +1307,5 @@ > + if (HasAudio()) { > + UpdateReceivedNewData(TrackType::kAudioTrack); > + } > + if (HasVideo()) { > + videoti = mVideo.mTimeRanges; this can be moved up now and merge with the previous HasVideo() @@ +1310,5 @@ > + if (HasVideo()) { > + videoti = mVideo.mTimeRanges; > + } > + if (HasAudio()) { > + audioti = mAudio.mTimeRanges; Same for this one

To be merge with P6-4 patch

Comment on attachment 8625049 [details] [diff] [review] P6-3. Cache SourceBuffer buffered ranges. Review of attachment 8625049 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/mediasource/TrackBuffer.cpp @@ +262,5 @@ > + ProxyMediaCall(mParentDecoder->GetReader()->TaskQueue(), this, __func__, > + &TrackBuffer::UpdateBufferedRanges, > + mLastAppendRange, /* aNotifyParent */ true) > + ->Then(mParentDecoder->GetReader()->TaskQueue(), __func__, > + [self] (bool) { Don't need to declare these args, here and below. @@ +288,5 @@ > +} > + > +nsRefPtr<TrackBuffer::BufferedRangesUpdatedPromise> > +TrackBuffer::UpdateBufferedRanges(Interval<int64_t> aByteRange, bool aNotifyParent) > +{ This should assert OnTaskQueue. @@ +884,5 @@ > + ProxyMediaCall(mParentDecoder->GetReader()->TaskQueue(), this, __func__, > + &TrackBuffer::UpdateBufferedRanges, > + Interval<int64_t>(), /* aNotifyParent */ true) > + ->Then(mParentDecoder->GetReader()->TaskQueue(), __func__, > + [self] (bool) { Here and below as well. ::: dom/media/mediasource/TrackBuffer.h @@ +24,5 @@ > class MediaSourceDecoder; > class MediaByteBuffer; > > +using media::TimeIntervals; > +using media::Interval; In general, we're not supposed to use |using| statements in header files, but this is probably fine.

Comment on attachment 8625050 [details] [diff] [review] P6-4. Cache SourceBuffer buffered ranges. Review of attachment 8625050 [details] [diff] [review]: ----------------------------------------------------------------- Apparently most of this goes away in the next patch, so...

Comment on attachment 8625398 [details] [diff] [review] Returns proper time ranges when endOfStream was called. Review of attachment 8625398 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/mediasource/MediaSourceDecoder.cpp @@ +103,5 @@ > +MediaSourceDecoder::GetBuffered() > +{ > + if (!NS_IsMainThread()) { > + return MediaDecoder::GetBuffered(); > + } Please assert against this case.

Comment on attachment 8625399 [details] [diff] [review] Make it work with about:media plugin Review of attachment 8625399 [details] [diff] [review]: ----------------------------------------------------------------- rs=me

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e06368496d2 https://hg.mozilla.org/integration/mozilla-inbound/rev/2d2cefa397dc https://hg.mozilla.org/integration/mozilla-inbound/rev/41ffc9a9ac48 https://hg.mozilla.org/integration/mozilla-inbound/rev/a8bd7a0d2aea https://hg.mozilla.org/integration/mozilla-inbound/rev/6aa5fa1d318e https://hg.mozilla.org/integration/mozilla-inbound/rev/6776ce74b9e5 https://hg.mozilla.org/integration/mozilla-inbound/rev/e02dd312d622 https://hg.mozilla.org/integration/mozilla-inbound/rev/a369cfb95b59

Backed out for frequent media test failures, mostly e10s. https://treeherder.mozilla.org/logviewer.html#?job_id=11029470&repo=mozilla-inbound https://treeherder.mozilla.org/logviewer.html#?job_id=11028211&repo=mozilla-inbound https://treeherder.mozilla.org/logviewer.html#?job_id=11029938&repo=mozilla-inbound etc. https://hg.mozilla.org/integration/mozilla-inbound/rev/85029878321b

bholley Id be very keen to split this bug in three parts (SafeReadAt and the changes to MediaSourceDecoder) If this one can't be landed quickly

(In reply to Jean-Yves Avenard [:jya] from comment #86) > bholley Id be very keen to split this bug in three parts (SafeReadAt and the > changes to MediaSourceDecoder) > If this one can't be landed quickly I'm closing in on the last bug. Hope to get there today.

Comment on attachment 8626825 [details] [diff] [review] Implement SilentReadAt - Adendum. v1 Review of attachment 8626825 [details] [diff] [review]: ----------------------------------------------------------------- Doh!

https://hg.mozilla.org/integration/mozilla-inbound/rev/40a894ed5740 https://hg.mozilla.org/integration/mozilla-inbound/rev/b043b3ec9ccb https://hg.mozilla.org/integration/mozilla-inbound/rev/0810020721e7 https://hg.mozilla.org/integration/mozilla-inbound/rev/94a10b4a2d1e https://hg.mozilla.org/integration/mozilla-inbound/rev/36d06eceb29d https://hg.mozilla.org/integration/mozilla-inbound/rev/0d17eedad8a9 https://hg.mozilla.org/integration/mozilla-inbound/rev/f5b56f26141b https://hg.mozilla.org/integration/mozilla-inbound/rev/9fb9dbd7a0dd

https://hg.mozilla.org/mozilla-central/rev/40a894ed5740 https://hg.mozilla.org/mozilla-central/rev/b043b3ec9ccb https://hg.mozilla.org/mozilla-central/rev/0810020721e7 https://hg.mozilla.org/mozilla-central/rev/94a10b4a2d1e https://hg.mozilla.org/mozilla-central/rev/36d06eceb29d https://hg.mozilla.org/mozilla-central/rev/0d17eedad8a9 https://hg.mozilla.org/mozilla-central/rev/f5b56f26141b https://hg.mozilla.org/mozilla-central/rev/9fb9dbd7a0dd

This or bug 1177501 caused bug 1180214.

This was apparently partially reverted in https://hg.mozilla.org/mozilla-central/rev/ac46c38d047f