882537 - MFCreateSourceReaderFromByteStream() sometimes fails on Windows 7

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

MFCreateSourceReaderFromByteStream() sometimes fails. This causes the media load to abort and thrown an error event.

I can reproduce this on Windows 7 when I run test_too_many_elements on an MP3/M4A file, after about 60 decoders have reached loadeddata.

MFCreateSourceReaderFromByteStream() failing is also the reason for the random failure in the WMF backend in bug 707777 and bug 857424. I'm not sure if that's caused by hitting this 60-decoder limit, or for some other reason.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: Force decoders to be destroyed in media tests

I've given up trying to do a clever solution, it's taking too much time to stabilize. So instead of trying to make the WMF backend release its resource when its document becomes inactive, this patch just changes the tests to force the decoders to be released. This fixes the failure that becomes almost perma-orange when I land the patch from bug 875644 (test_bug493187 timing out due to the decoder not being able to start up).

I get a pretty frequent failure with this patch:

https://tbpl.mozilla.org/?tree=Try&rev=a44b46c3bc1d

149269 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_play_events.html | Last event should be canplaythrough for http://mochi.test:8888/tests/content/media/test/gizmo.mp4 - got playing, expected canplaythrough 

But that's fixed by Roc's patches in bug 879717 and bug 833731, but those have other oranges that need to be fixed too?

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I sympathise with your frustration here but patching all these tests worries me a lot. We might miss bugs related to video elements lingering in the bfcache (of which we've had a few!). I suggest a couple of things:

1) Always-on instrumentation to track the number of native decoders we currently are keeping alive --- dumping those stats when we fail to create a native object. Then, when we hit failures, we'll be able to tell whether efforts to reduce those numbers worked or not. (We might even decide to add those to about:memory at some point.)

2) When we fail to create a native object, dispatch a runnable to the main thread that does
    nsJSContext::GarbageCollectNow(JS::gcreason::MEM_PRESSURE,
                                   nsJSContext::NonIncrementalGC,
                                   nsJSContext::NonCompartmentGC,
                                   nsJSContext::ShrinkingGC);
    nsJSContext::CycleCollectNow();
(see nsJSEnvironmentObserver::Observe), and when that has completed, retry creating the native decoder. It won't really have completed synchronously, since MediaDecoder shutdown is asynchronous, so I suggest keeping a count of the number of MediaDecoders that have entered MediaDecoder::Shutdown but not been destroyed yet, and delay retrying creation of the native decoder until that count is zero.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

It seems that there's still a bug somewhere in my IMFByteStream implementation. I took Chris Double's patches from bug 860599 to create a network proxy, and used that to feed data into WMF, and the orange that happens when I include the patches for bug 875644 go away:

https://tbpl.mozilla.org/?tree=Try&rev=929715f9fb03

Presumably the bug in WMFByteStream is causing connections/resources not to close properly, and that's causing resource contention issues, which causes this failure.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

I can reliably reproduce this on Windows 7 by constantly creating autoplaying short audio elements:

STR:
1. Set pref media.directshow.enabled to false.
2. Load http://people.mozilla.org/~cpearce/audio_stress_test.html
3. Lots of short sounds will play, eventually they will fail to be created and error will be reported at the top of the page.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

In theory this is fixed by bug 1057879...