https://hg.mozilla.org/mozilla-central/annotate/0b69d304f861d0038fb78f1d52b0f5d13ef7c6fe/dom/media/platforms/wrappers/FuzzingWrapper.cpp#l239 The task queue of DecoderCallbackFuzzingWrapper is different from that of the reader. |mCallback->ReleaseMediaResources| should be called on the reader's task queue instead of that of DecoderCallbackFuzzingWrapper.

Hi jya, It seems wrong to call |mCallback->ReleaseMediaResources| on the task queue of DecoderCallbackFuzzingWrapper.

Gerald would know this more than I

This wrapper uses an internal queue to avoid having to use a lock, and to make sure all calls into the wrapper are forwarded in the same order. This is mostly important for the Output/Error/InputExhausted/DrainComplete methods (as they are inter-dependent), but I just applied this usage to ReleaseMediaResources as well for consistency. ReleaseMediaResources is part of the MediaDataDecoderCallback interface. The documentation there reads: "Implementation is threadsafe, and can be called on any thread." https://hg.mozilla.org/mozilla-central/annotate/0b69d304f861d0038fb78f1d52b0f5d13ef7c6fe/dom/media/platforms/PlatformDecoderModule.h#l112 So I assumed it was necessary for me to use my own queue when implementing this interface (to make it thread-safe), and also that it was safe when calling the original callback from my own queue. Is that documentation incorrect?

I guess it should be rephrased to "Implementation must be threadsafe". https://hg.mozilla.org/mozilla-central/file/0b69d304f861/dom/media/MediaFormatReader.h#l174 DecoderCallback::ReleaseMediaResources() calls mReader->ReleaseMediaResources() which must happen on the reader's task queue. According to the document, it is the responsibility of DecoderCallback to ensure the implementation is thread-safe.

(In reply to Gerald Squelart [:gerald] from comment #3) > This wrapper uses an internal queue to avoid having to use a lock, and to > make sure all calls into the wrapper are forwarded in the same order. > This is mostly important for the Output/Error/InputExhausted/DrainComplete > methods (as they are inter-dependent), but I just applied this usage to > ReleaseMediaResources as well for consistency. I see. Then we should fix DecoderCallback per comment 4.

(In reply to JW Wang [:jwwang] from comment #5) > I see. Then we should fix DecoderCallback per comment 4. Or fix MediaFormatReader::ReleaseMediaResources() to dispatch itself to the reader queue, the same way MediaFormatReader::Output and other do; this would be more consistent overall. In any case, this bug here is probably "invalid" and a new one should be created; or you could just change the title.

Right. I open bug 1214073 to fix MediaDecoderReader::ReleaseMediaResources. I will close this bug as invalid. Thanks!