1203374 - Make MediaDecoder::CanPlayThrough run on the main thread

Status: RESOLVED FIXED

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

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1203374. Part 1 - extract the code of computing canplaythrough so it is reusable.

Bug 1203374. Part 1 - extract the code of computing canplaythrough so it is reusable.
1. extract MediaDecoder::Statistics to its own file.
2. sort out include orders of MediaDecoderStateMachine.cpp

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1203374. Part 2 - duplicate the implementation of MediaDecoder::CanPlayThrough so MDSM can call its own CanPlayThrough() on its own thread.

Bug 1203374. Part 2 - duplicate the implementation of MediaDecoder::CanPlayThrough so MDSM can call its own CanPlayThrough() on its own thread.

Comment 3

[Jean-Yves Avenard [:jya]]

Comment on attachment 8659066 [details]
MozReview Request: Bug 1203374. Part 2 - duplicate the implementation of MediaDecoder::CanPlayThrough so MDSM can call its own CanPlayThrough() on its own thread.

https://reviewboard.mozilla.org/r/18749/#review16783

r=me with comment addressed.

::: dom/media/MediaDecoderStateMachine.cpp:2802
(Diff revision 1)
> +  ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());

Do you need to hold the monitor here?
after all ; GetStatistics() is thread-safe already

and IsRealTime() is only ever accessed on the MDSM task queue

Comment 4

[Jean-Yves Avenard [:jya]]

Comment on attachment 8659065 [details]
MozReview Request: Bug 1203374. Part 1 - extract the code of computing canplaythrough so it is reusable.

https://reviewboard.mozilla.org/r/18747/#review16785

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

https://reviewboard.mozilla.org/r/18749/#review16783

> Do you need to hold the monitor here?
> after all ; GetStatistics() is thread-safe already
> 
> and IsRealTime() is only ever accessed on the MDSM task queue

It is just a convention that all(most) MDSM functions are run in the task queue with the monitor held. When we finish the refactoring, we will be able to remove the monitor from all functions of MDSM.

Thanks for the review.

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a21b48a97209

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d849c8c7a995
https://hg.mozilla.org/integration/mozilla-inbound/rev/d27278fc308e

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d849c8c7a995
https://hg.mozilla.org/mozilla-central/rev/d27278fc308e