Remove usage of decoder monitor from MediaOmxCommonDecoder

RESOLVED FIXED in Firefox 44

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

unspecified
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
https://hg.mozilla.org/mozilla-central/file/acdb22976ff8/dom/media/omx/MediaOmxCommonDecoder.cpp#l49

This must be done before bug 1208934 because CheckDecoderCanOffloadAudio() is called from multiple threads. We will make all methods of MediaOmxCommonDecoder run on the main thread in this bug.
(Assignee)

Updated

3 years ago
Assignee: nobody → jwwang
Blocks: 1208934, 1146482
(Assignee)

Updated

3 years ago
Depends on: 1053186
(Assignee)

Comment 1

3 years ago
CheckDecoderCanOffloadAudio() is introduced in bug 1053186. Make sure not to cause regression when fixing this bug.
(Assignee)

Comment 3

3 years ago
Created attachment 8668226 [details]
MozReview Request: Bug 1209864. Part 1 - make all methods run on the main thread and remove usage of the decoder monitor.

Bug 1209864. Part 1 - make all methods run on the main thread and remove usage of the decoder monitor.
Attachment #8668226 - Flags: review?(roc)
(Assignee)

Comment 4

3 years ago
Created attachment 8668227 [details]
MozReview Request: Bug 1209864. Part 2 - remove unused code. r=sotaro.

Bug 1209864. Part 2 - remove unused code.
Attachment #8668227 - Flags: review?(roc)
(Assignee)

Comment 5

3 years ago
It looks like roc is on PTO.
(Assignee)

Comment 6

3 years ago
Comment on attachment 8668226 [details]
MozReview Request: Bug 1209864. Part 1 - make all methods run on the main thread and remove usage of the decoder monitor.

Bug 1209864. Part 1 - make all methods run on the main thread and remove usage of the decoder monitor.
Attachment #8668226 - Flags: review?(roc) → review?(sotaro.ikeda.g)
(Assignee)

Comment 7

3 years ago
Comment on attachment 8668227 [details]
MozReview Request: Bug 1209864. Part 2 - remove unused code. r=sotaro.

Bug 1209864. Part 2 - remove unused code.
Attachment #8668227 - Flags: review?(roc) → review?(sotaro.ikeda.g)
Comment on attachment 8668226 [details]
MozReview Request: Bug 1209864. Part 1 - make all methods run on the main thread and remove usage of the decoder monitor.

https://reviewboard.mozilla.org/r/20905/#review19041
Attachment #8668226 - Flags: review?(sotaro.ikeda.g) → review+
Comment on attachment 8668227 [details]
MozReview Request: Bug 1209864. Part 2 - remove unused code. r=sotaro.

https://reviewboard.mozilla.org/r/20907/#review19039
Attachment #8668227 - Flags: review?(sotaro.ikeda.g) → review+
Comment on attachment 8668226 [details]
MozReview Request: Bug 1209864. Part 1 - make all methods run on the main thread and remove usage of the decoder monitor.

https://reviewboard.mozilla.org/r/20905/#review19055

::: dom/media/omx/MediaOmxCommonDecoder.cpp:91
(Diff revision 1)
> +    GetStateMachine()->DispatchAudioOffloading(false);

It might be better to add a comment why the call is necessary. Because MDSM's default value is false.

::: dom/media/omx/MediaOmxCommonDecoder.cpp:102
(Diff revision 1)
> +    GetStateMachine()->DispatchAudioOffloading(false);

Same as above.
Attachment #8668226 - Flags: review+
(Assignee)

Comment 11

3 years ago
Comment on attachment 8668226 [details]
MozReview Request: Bug 1209864. Part 1 - make all methods run on the main thread and remove usage of the decoder monitor.

Bug 1209864. Part 1 - make all methods run on the main thread and remove usage of the decoder monitor.
Attachment #8668226 - Flags: review?(sotaro.ikeda.g)
(Assignee)

Comment 12

3 years ago
Comment on attachment 8668227 [details]
MozReview Request: Bug 1209864. Part 2 - remove unused code. r=sotaro.

Bug 1209864. Part 2 - remove unused code. r=sotaro.
Attachment #8668227 - Attachment description: MozReview Request: Bug 1209864. Part 2 - remove unused code. → MozReview Request: Bug 1209864. Part 2 - remove unused code. r=sotaro.
Comment on attachment 8668226 [details]
MozReview Request: Bug 1209864. Part 1 - make all methods run on the main thread and remove usage of the decoder monitor.

https://reviewboard.mozilla.org/r/20905/#review19077
(Assignee)

Comment 14

3 years ago
Thanks for the review!
(Assignee)

Updated

3 years ago
Attachment #8668226 - Flags: review?(sotaro.ikeda.g)
https://hg.mozilla.org/mozilla-central/rev/8bbb629f0d54
https://hg.mozilla.org/mozilla-central/rev/1be141d7888c
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.