The default bug view has changed. See this FAQ.

Simplify MediaStreamGraph

RESOLVED FIXED in mozilla17

Status

()

Core
Audio/Video
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
mozilla17
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments)

This is in preparation for bug 779715.
Blocks: 779715
No longer depends on: 779715
Created attachment 648210 [details] [diff] [review]
Part 1: simplify invariants around when messages are processed
Attachment #648210 - Flags: review?(rjesup)
Created attachment 648211 [details] [diff] [review]
Part 2: don't block for no consumers
Attachment #648211 - Flags: review?(rjesup)
Created attachment 648212 [details] [diff] [review]
Part 3: don't shut down MediaStreams on the MediaStreamGraph thread during forced shutdown
Attachment #648212 - Flags: review?(rjesup)
Created attachment 648213 [details] [diff] [review]
Part 4: Fix lock ordering inversion when running control messages during a forced shutdown
Attachment #648213 - Flags: review?(rjesup)
Created attachment 648214 [details] [diff] [review]
Part 5: Add a comment explaining more about how MediaStream lifetimes are managed
Attachment #648214 - Flags: review?(rjesup)
Created attachment 648597 [details] [diff] [review]
Part 6: Check that Destroy is the last message sent from the main thread for a stream

The IsDestroyed() getter will come in handy later for future patches, too.
Attachment #648597 - Flags: review?(rjesup)
Comment on attachment 648210 [details] [diff] [review]
Part 1: simplify invariants around when messages are processed

Review of attachment 648210 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/MediaStreamGraph.cpp
@@ +895,3 @@
>      LOG(PR_LOG_DEBUG, ("Media graph %p computed blocking for interval %f to %f",
> +                       this, MediaTimeToSeconds(mStateComputedTime),
> +                       MediaTimeToSeconds(end)));                       

trailing whitespace
Attachment #648210 - Flags: review?(rjesup) → review+

Updated

5 years ago
Attachment #648211 - Flags: review?(rjesup) → review+

Updated

5 years ago
Attachment #648212 - Flags: review?(rjesup) → review+

Updated

5 years ago
Attachment #648213 - Flags: review?(rjesup) → review+
Comment on attachment 648214 [details] [diff] [review]
Part 5: Add a comment explaining more about how MediaStream lifetimes are managed

Review of attachment 648214 [details] [diff] [review]:
-----------------------------------------------------------------

I like the lifetime description!  Everything should have one... :-)
Attachment #648214 - Flags: review?(rjesup) → review+
Comment on attachment 648597 [details] [diff] [review]
Part 6: Check that Destroy is the last message sent from the main thread for a stream

Review of attachment 648597 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me - I debugged through one these quite a bit; this makes it easier to try to flag and trigger on
Attachment #648597 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/51de5c54e500
https://hg.mozilla.org/integration/mozilla-inbound/rev/28860e96d78d
https://hg.mozilla.org/integration/mozilla-inbound/rev/cecd42ed7943
https://hg.mozilla.org/integration/mozilla-inbound/rev/409c0d581dcd
https://hg.mozilla.org/integration/mozilla-inbound/rev/cdf37b6a641a
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e9222b7cfcc
https://hg.mozilla.org/mozilla-central/rev/51de5c54e500
https://hg.mozilla.org/mozilla-central/rev/28860e96d78d
https://hg.mozilla.org/mozilla-central/rev/cecd42ed7943
https://hg.mozilla.org/mozilla-central/rev/409c0d581dcd
https://hg.mozilla.org/mozilla-central/rev/cdf37b6a641a
https://hg.mozilla.org/mozilla-central/rev/2e9222b7cfcc
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Depends on: 790854
You need to log in before you can comment on or make changes to this bug.