Closed
Bug 1163467
Opened 9 years ago
Closed 9 years ago
Move DecodedStreamData from MediaDecoder to MediaDecoderStateMachine
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: jwwang, Assigned: jwwang)
References
Details
Attachments
(6 files)
3.70 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
4.86 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
3.89 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
10.33 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
8.00 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
21.07 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
Per comment at https://hg.mozilla.org/mozilla-central/file/617dbce26726/dom/media/MediaDecoderStateMachine.cpp#l456.
Assignee | ||
Comment 1•9 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6bb9223e4f47
Assignee | ||
Comment 2•9 years ago
|
||
Part 1 - refactor DecodedStream::RecreateData() which is responsible for destroying DecodedStreamData if necessary.
Assignee | ||
Comment 3•9 years ago
|
||
Part 2 - rename MediaDecoder::UpdateDecodedStream to MediaDecoder::UpdateStreamBlockingForPlayState.
Attachment #8610450 -
Flags: review?(roc)
Assignee | ||
Comment 4•9 years ago
|
||
Part 3 - move AddOutputStream code below MediaDecoder::Load(). Thanks to tail dispatcher, MDSM cycles won't start running until current event cycle is finished. We are safe to move AddOutputStream code below MediaDecoder::Load() call and simplify the code in MediaDecoder::AddOutputStream().
Attachment #8610453 -
Flags: review?(roc)
Assignee | ||
Comment 5•9 years ago
|
||
Part 4 - move MediaDecoder::UpdateStreamBlockingForStateMachinePlaying to MediaDecoderStateMachine.
Attachment #8610455 -
Flags: review?(roc)
Assignee | ||
Comment 6•9 years ago
|
||
Part 5 - move MediaDecoder::UpdateStreamBlockingForPlayState to MediaDecoderStateMachine.
Attachment #8610456 -
Flags: review?(roc)
Assignee | ||
Comment 7•9 years ago
|
||
Part 6 - refactor MediaDecoder::AddOutputStream and move related code to MediaDecoderStateMachine.
Attachment #8610458 -
Flags: review?(roc)
Comment on attachment 8610449 [details] [diff] [review] 1163467_part1_refactor_RecreateData-v1.patch Review of attachment 8610449 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/DecodedStream.cpp @@ +230,2 @@ > > + aGraph = aGraph ? aGraph : mData->mStream->Graph(); if (!aGraph) { aGraph = mData->mStream->Graph(); }
Attachment #8610449 -
Flags: review?(roc) → review+
Attachment #8610450 -
Flags: review?(roc) → review+
Attachment #8610453 -
Flags: review?(roc) → review+
Attachment #8610455 -
Flags: review?(roc) → review+
Attachment #8610456 -
Flags: review?(roc) → review+
Comment on attachment 8610458 [details] [diff] [review] 1163467_part6_refactor_MediaDecoder_AddOutputStream-v1.patch Review of attachment 8610458 [details] [diff] [review]: ----------------------------------------------------------------- Excellent!
Attachment #8610458 -
Flags: review?(roc) → review+
Assignee | ||
Comment 10•9 years ago
|
||
Thanks for the review!
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0df38f5ec5b https://hg.mozilla.org/integration/mozilla-inbound/rev/079326caec09 https://hg.mozilla.org/integration/mozilla-inbound/rev/092ce739460d https://hg.mozilla.org/integration/mozilla-inbound/rev/8563a422f957 https://hg.mozilla.org/integration/mozilla-inbound/rev/68285bdfa4d7 https://hg.mozilla.org/integration/mozilla-inbound/rev/1bda9d3fec3b
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f0df38f5ec5b https://hg.mozilla.org/mozilla-central/rev/079326caec09 https://hg.mozilla.org/mozilla-central/rev/092ce739460d https://hg.mozilla.org/mozilla-central/rev/8563a422f957 https://hg.mozilla.org/mozilla-central/rev/68285bdfa4d7 https://hg.mozilla.org/mozilla-central/rev/1bda9d3fec3b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment 13•9 years ago
|
||
This added more main-thread monitor-dependent methods to MediaDecoderStateMachine. Please don't do that in the future!
Assignee | ||
Comment 14•9 years ago
|
||
Can you indicate which piece of the code creating more dependency? This bug is basically move part of the stream-related code to another file.
Comment 15•9 years ago
|
||
See bug 1178691. Specifically, we now access thread-bound state in IsPlaying etc.
You need to log in
before you can comment on or make changes to this bug.
Description
•