Closed
Bug 1070216
Opened 10 years ago
Closed 9 years ago
Add support for MediaStream constructors (from MediaStreamTracks)
Categories
(Core :: Audio/Video, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: jesup, Assigned: pehrsons)
References
(Blocks 3 open bugs)
Details
(Keywords: dev-doc-needed)
Attachments
(11 files)
40 bytes,
text/x-review-board-request
|
roc
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
roc
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
jib
:
review+
padenot
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
jib
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
roc
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
jib
:
review+
|
Details |
MozReview Request: Bug 1070216 - Assert main MediaEngine APIs are called on the owning thread. r?jib
40 bytes,
text/x-review-board-request
|
jib
:
review+
|
Details |
+++ This bug was initially created as a clone of Bug #910249 +++
One of the things we've never implemented is the ability to take tracks from two MediaStreams and combine them using a MediaStream constructor to make a third MediaStream. For example, and video track from a getUserMedia stream, and an audio track from a second (audio processed through WebAudio, for example).
Note: not explicitly Webrtc, so in Video/Audio
Comment 1•10 years ago
|
||
Unfeasible if MediaStream.addTrack() is not implemented: #1103188
Assignee | ||
Comment 2•10 years ago
|
||
This should be simple with a default constructor and addTrack() (blocking this bug).
Comment 3•10 years ago
|
||
This will be very useful for bug 1156472, see comment 1. In fact, it's kind of blocking it.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → pehrsons
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•9 years ago
|
||
Bug 1070216 - Implement MediaStream Constructors. r?smaug,roc
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8665902 [details]
MozReview Request: Bug 1070216 - Split DOMMediaStream::InitStreamCommon into three. r?roc
Paul, if you check the diff of this patch you'll see that all three constructors are doing `MediaStreamGraph::GetInstance(SYSTEM_THREAD_DRIVER, AudioChannel::Normal);`
Is there something to think about here? Could anything go wrong if, let's say, I add a track belonging to another graph than I got from GetInstance()?
Attachment #8665902 -
Flags: feedback?(padenot)
Comment 7•9 years ago
|
||
(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #6)
> Comment on attachment 8665902 [details]
> MozReview Request: Bug 1070216 - Implement MediaStream Constructors.
> r?smaug,roc
>
> Paul, if you check the diff of this patch you'll see that all three
> constructors are doing `MediaStreamGraph::GetInstance(SYSTEM_THREAD_DRIVER,
> AudioChannel::Normal);`
>
> Is there something to think about here? Could anything go wrong if, let's
> say, I add a track belonging to another graph than I got from GetInstance()?
You should get the graph from the stream, and maybe add a debug assert and early return in release just to make sure.
For now, we should throw if you try to put two streams from different graphs together. In the future, we will need to support connections between graphs, but I don't think it should block this.
Updated•9 years ago
|
Attachment #8665902 -
Flags: feedback?(padenot)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8665902 [details]
MozReview Request: Bug 1070216 - Split DOMMediaStream::InitStreamCommon into three. r?roc
Bug 1070216 - Split DOMMediaStream::InitStreamCommon into three. r?roc
This allows us to initiate a DOMMediaStream into three states:
* One that has input, owned and playback streams, for data producers
like gUM or RTCPeerConnection.
* One with owned and playback streams, for cloned DOM streams.
If a cloned DOM stream has an empty input stream connected to its
owned stream, both are regarded as not having current data.
* One with only a playback stream, for when it has been created with the
default constructor from JS. Its track set can only be changed by
addTrack() and removeTrack() which when called only affect mPlaybackStream.
Attachment #8665902 -
Attachment description: MozReview Request: Bug 1070216 - Implement MediaStream Constructors. r?smaug,roc → MozReview Request: Bug 1070216 - Split DOMMediaStream::InitStreamCommon into three. r?roc
Attachment #8665902 -
Flags: review?(roc)
Assignee | ||
Comment 9•9 years ago
|
||
Bug 1070216 - constify DOMMediaStream::Get[Audio/Video]Tracks(). r?roc
Attachment #8668793 -
Flags: review?(roc)
Assignee | ||
Comment 10•9 years ago
|
||
Bug 1070216 - Guard against adding a track owned by one MSG to a stream owned by another. r?padenot
Attachment #8668794 -
Flags: review?(padenot)
Assignee | ||
Comment 11•9 years ago
|
||
Bug 1070216 - Implement MediaStream constructors. r?smaug,jib
Attachment #8668795 -
Flags: review?(jib)
Attachment #8668795 -
Flags: review?(bugs)
Assignee | ||
Comment 12•9 years ago
|
||
Bug 1070216 - Make it possible to disable AudioStreamAnalyser's debug canvas. r?padenot
Attachment #8668796 -
Flags: review?(padenot)
Assignee | ||
Comment 13•9 years ago
|
||
Bug 1070216 - Let AudioStreamAnalyser accept streams with no tracks. r?padenot
Attachment #8668797 -
Flags: review?(padenot)
Assignee | ||
Comment 14•9 years ago
|
||
Bug 1070216 - Break out createOscillatorStream from test_gUM_addTrackRemoveTrack.html to test framework. r?padenot
Attachment #8668798 -
Flags: review?(padenot)
Assignee | ||
Comment 15•9 years ago
|
||
Bug 1070216 - Test MediaStream Constructors. r?jib
Attachment #8668799 -
Flags: review?(jib)
Updated•9 years ago
|
Attachment #8668795 -
Flags: review?(jib) → review+
Comment 16•9 years ago
|
||
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot
https://reviewboard.mozilla.org/r/21037/#review18927
::: dom/media/DOMMediaStream.cpp:469
(Diff revision 1)
> +/* static */ already_AddRefed<DOMMediaStream>
> +DOMMediaStream::Constructor(const dom::GlobalObject& aGlobal,
> + const dom::Sequence<OwningNonNull<MediaStreamTrack>>& aTracks,
> + ErrorResult& aRv)
The other two constructors can be implemented with this one.
::: dom/media/DOMMediaStream.cpp:716
(Diff revision 1)
> +DOMMediaStream::CreateEmptyStream(nsIDOMWindow* aWindow)
> +{
> + nsRefPtr<DOMMediaStream> newStream = new DOMMediaStream();
> + newStream->mWindow = aWindow;
> + return newStream.forget();
> +}
Then this inlines.
Assignee | ||
Comment 17•9 years ago
|
||
https://reviewboard.mozilla.org/r/21037/#review18927
> The other two constructors can be implemented with this one.
Not quite, because we want the new playback stream to be created in the same MediaStreamGraph as the tracks we're adding.
With the default constructor we don't know which tracks will be added so we guess at the MSG for the normal AudioChannel (there's one MSG per AudioChannel per my discussion on IRC with padenot yesterday). For the other two constructors we can figure out which MSG to use from the stream/tracks passed in. Streams from different MSGs cannot mix.
I should perhaps let padenot review this one as well..
Assignee | ||
Comment 18•9 years ago
|
||
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot
Bug 1070216 - Implement MediaStream constructors. r?smaug,jib
Attachment #8668795 -
Flags: review?(padenot)
Comment 19•9 years ago
|
||
https://reviewboard.mozilla.org/r/21037/#review18927
> Not quite, because we want the new playback stream to be created in the same MediaStreamGraph as the tracks we're adding.
>
> With the default constructor we don't know which tracks will be added so we guess at the MSG for the normal AudioChannel (there's one MSG per AudioChannel per my discussion on IRC with padenot yesterday). For the other two constructors we can figure out which MSG to use from the stream/tracks passed in. Streams from different MSGs cannot mix.
>
> I should perhaps let padenot review this one as well..
But you do that in the last constructor as well if the passed-in tracks sequence is empty.
Assignee | ||
Comment 20•9 years ago
|
||
https://reviewboard.mozilla.org/r/21037/#review18927
> But you do that in the last constructor as well if the passed-in tracks sequence is empty.
Yeah, I could optimize that piece (empty tracks sequence) to use the default constructor, but it doesn't get rid of the need for CreateEmptyStream().
Comment 21•9 years ago
|
||
https://reviewboard.mozilla.org/r/21037/#review18927
> Yeah, I could optimize that piece (empty tracks sequence) to use the default constructor, but it doesn't get rid of the need for CreateEmptyStream().
No, the other way around. Implement the default constructor with the last one:
/* static */ already_AddRefed<DOMMediaStream>
DOMMediaStream::Constructor(const GlobalObject& aGlobal, ErrorResult& aRv)
{
return Constructor(aGlobal, Sequence<OwningNonNull<MediaStreamTrack>>(), aRv);
}
The second one can be as well. Also, the dom:: prefixes are unneeded.
> Then this inlines.
Once the constructor implementations are merged, this is used from one place, and can be inlined.
Comment 22•9 years ago
|
||
Comment on attachment 8668799 [details]
MozReview Request: Bug 1070216 - Test MediaStream Constructors. r?jib
https://reviewboard.mozilla.org/r/21045/#review18929
::: dom/media/tests/mochitest/test_getUserMedia_mediaStreamConstructors.html:49
(Diff revision 1)
> + checkMediaStreamContains(stream, [audioTrack, videoTrack],
> + "List constructed audio and video tracks");
You don't have complete code coverage here with three different constructor implementations. E.g. you'd want to repeat this test with an empty array for instance.
::: dom/media/tests/mochitest/test_getUserMedia_mediaStreamConstructors.html:52
(Diff revision 1)
> + var test = createMediaElement('video', 'testCopyConstructor');
> + var playback = new MediaStreamPlayback(test, stream);
> + return playback.playMedia(false).then(() => gUMStream.stop());
Boilerplate x 3. Helper?
::: dom/media/tests/mochitest/test_getUserMedia_mediaStreamConstructors.html:73
(Diff revision 1)
> + return analyser.waitForAnalysisSuccess(array =>
> + array[analyser.binIndexForFrequency(1000)] < 50 &&
> + array[analyser.binIndexForFrequency(5000)] < 50 &&
> + array[analyser.binIndexForFrequency(10000)] < 50)
> + .then(() => analyser.disableDebugCanvas());
unusual indent
Attachment #8668799 -
Flags: review?(jib) → review+
Assignee | ||
Comment 23•9 years ago
|
||
https://reviewboard.mozilla.org/r/21037/#review18927
> No, the other way around. Implement the default constructor with the last one:
>
> /* static */ already_AddRefed<DOMMediaStream>
> DOMMediaStream::Constructor(const GlobalObject& aGlobal, ErrorResult& aRv)
> {
> return Constructor(aGlobal, Sequence<OwningNonNull<MediaStreamTrack>>(), aRv);
> }
>
> The second one can be as well. Also, the dom:: prefixes are unneeded.
Oh. Yeah that works :-)
Updated•9 years ago
|
Attachment #8668794 -
Flags: review?(padenot) → review+
Comment 24•9 years ago
|
||
Comment on attachment 8668794 [details]
MozReview Request: Bug 1070216 - Guard against adding a track owned by one MSG to a stream owned by another. r?padenot
https://reviewboard.mozilla.org/r/21035/#review18941
::: dom/media/DOMMediaStream.cpp:484
(Diff revision 1)
> + if (mPlaybackStream->Graph() !=
Maybe we should notify authors in this case ?
Comment 25•9 years ago
|
||
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot
https://reviewboard.mozilla.org/r/21037/#review18943
Attachment #8668795 -
Flags: review?(padenot)
Updated•9 years ago
|
Attachment #8668796 -
Flags: review?(padenot) → review+
Comment 26•9 years ago
|
||
Comment on attachment 8668796 [details]
MozReview Request: Bug 1070216 - Make it possible to disable AudioStreamAnalyser's debug canvas. r?padenot
https://reviewboard.mozilla.org/r/21039/#review18945
Comment 27•9 years ago
|
||
Comment on attachment 8668797 [details]
MozReview Request: Bug 1070216 - Let AudioStreamAnalyser accept streams with no tracks. r?padenot
https://reviewboard.mozilla.org/r/21041/#review18947
Attachment #8668797 -
Flags: review?(padenot) → review+
Comment 28•9 years ago
|
||
Comment on attachment 8668798 [details]
MozReview Request: Bug 1070216 - Break out createOscillatorStream from test_gUM_addTrackRemoveTrack.html to test framework. r?padenot
https://reviewboard.mozilla.org/r/21043/#review18949
Attachment #8668798 -
Flags: review?(padenot) → review+
Assignee | ||
Comment 29•9 years ago
|
||
https://reviewboard.mozilla.org/r/21035/#review18941
> Maybe we should notify authors in this case ?
What do you suggest that makes sense?
"Combining tracks bound to different AudioChannels is not supported" ?
Comment 30•9 years ago
|
||
Yeah that'd be enough I guess, otherwise I can imagine a number of developers scratching their head for a long time before finding the reason.
Assignee | ||
Comment 31•9 years ago
|
||
jib, FYI, I'm running into this again (mediamanager:5 logs):
> 821575680[11313ae40]: virtual nsresult mozilla::MediaEngineRemoteVideoSource::Allocate(const dom::MediaTrackConstraints &, const mozilla::MediaEnginePrefs &, const nsString &)
> 821575680[11313ae40]: Video device 4097 allocated shared
> 821575680[11313ae40]: virtual nsresult mozilla::MediaEngineRemoteVideoSource::Stop(mozilla::SourceMediaStream *, mozilla::TrackID)
> 821575680[11313ae40]: Deallocate
> 821575680[11313ae40]: Video device 4097 deallocated
> 821575680[11313ae40]: virtual nsresult mozilla::MediaEngineRemoteVideoSource::Start(mozilla::SourceMediaStream *, TrackID)
> 821575680[11313ae40]: StartCapture failed
> 821575680[11313ae40]: Starting video failed , rv=-2147467259
I can't stop the played stream now since the constructors create DOMMediaStream and not DOMLocalMediaStream instances.
I looked a bit more into the issue - there is actually code to count the number of users of a device, but a device's lifetime is defined by Alloc()/Dealloc() while we do the counting in Start()/Stop() :(
Perhaps it's as simple as moving the counting to Alloc()/Dealloc(). I'll try.
Updated•9 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 32•9 years ago
|
||
(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #31)
> Perhaps it's as simple as moving the counting to Alloc()/Dealloc(). I'll try.
I did that, and it works.
However, now I'm running into a deadlock in non-e10s-mode.
* The deadlock consists of main thread (gUM()) waiting for a Mutex in CamerasChild,
vs,
* CamerasChild holding that mutex, waiting for a reply to SendReleaseCaptureDevice - with the receiving CamerasParent having dispatched the runnable responsible for responding to the main thread (see https://hg.mozilla.org/mozilla-central/annotate/2c1fb007137dcb68b1862a79553b53f1a34c99c3/dom/media/systemservices/CamerasParent.cpp#l648).
gcp, jib, how do you suggest solving this?
My two questions right now are,
Shouldn't IPC stuff run off the main thread?
Can we fix whatever issue we had on Mac with shutdown to make sure the CaptureDevice is not released on main thread?
In detail:
On main thread we have MediaManager::EnumerateRawDevices() wanting to talk to CamerasParent:
> * thread #1: tid = 0x39d4e7, 0x00007fff8fc8d166 libsystem_kernel.dylib`__psynch_mutexwait + 10, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
> frame #0: 0x00007fff8fc8d166 libsystem_kernel.dylib`__psynch_mutexwait + 10
> frame #1: 0x00007fff94719696 libsystem_pthread.dylib`_pthread_mutex_lock + 480
> frame #2: 0x0000000101331f9e libnss3.dylib`PR_Lock(lock=0x00000001156206c0) + 46 at ptsynch.c:177
> frame #3: 0x0000000101629197 XUL`mozilla::OffTheBooksMutex::Lock(this=0x00000001159e85b0) + 23 at BlockingResourceBase.cpp:382
> frame #4: 0x00000001037c0b57 XUL`mozilla::camera::CamerasChild::NumberOfCapabilities(mozilla::camera::CaptureEngine, char const*) [inlined] mozilla::BaseAutoLock<mozilla::Mutex>::BaseAutoLock(aLock=0x00000001159e85b0) + 39 at Mutex.h:164
> frame #5: 0x00000001037c0b4e XUL`mozilla::camera::CamerasChild::NumberOfCapabilities(mozilla::camera::CaptureEngine, char const*) [inlined] mozilla::BaseAutoLock<mozilla::Mutex>::BaseAutoLock(aLock=0x00000001159e85b0) at Mutex.h:161
> frame #6: 0x00000001037c0b4e XUL`mozilla::camera::CamerasChild::NumberOfCapabilities(this=0x00000001159e8540, aCapEngine=CameraEngine, deviceUniqueIdUTF8="0x1a11000005ac8510") + 30 at CamerasChild.cpp:245
> frame #7: 0x0000000103826389 XUL`mozilla::MediaEngineRemoteVideoSource::NumCapabilities(this=0x0000000115458b00) + 41 at MediaEngineRemoteVideoSource.cpp:350
> frame #8: 0x0000000103821af5 XUL`mozilla::MediaEngineCameraVideoSource::GetBestFitnessDistance(this=0x0000000115458b00, aConstraintSets=0x00007fff5fbfcaa8, aDeviceId=0x00007fff5fbfc9e8) + 37 at MediaEngineCameraVideoSource.cpp:111
> frame #9: 0x000000010367b707 XUL`mozilla::MediaDevice::GetBestFitnessDistance(this=<unavailable>, aConstraintSets=0x00007fff5fbfcaa8) + 231 at MediaManager.cpp:535
> * frame #10: 0x00000001036f09f4 XUL`char const* mozilla::MediaConstraintsHelper::SelectSettings<mozilla::VideoDevice>(aConstraints=0x0000000107c9de28, aSources=0x00007fff5fbfcd90) + 276 at MediaTrackConstraints.h:148
> frame #11: 0x000000010368f78c XUL`void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_20, mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_21>(mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_20, mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_21)::Functors::Succeed(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&) [inlined] mozilla::SelectSettings(aConstraints=<unavailable>, aSources=<unavailable>) + 5 at MediaManager.cpp:1162
> frame #12: 0x000000010368f787 XUL`void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_20, mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_21>(mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_20, mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_21)::Functors::Succeed(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&) + 433 at MediaManager.cpp:2040
> frame #13: 0x000000010368f5d6 XUL`void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::GetUserMedia(this=0x0000000112c6e000, result=<unavailable>)::$_20, mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_21>(mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_20, mozilla::MediaManager::GetUserMedia(nsPIDOMWindow*, mozilla::dom::MediaStreamConstraints const&, nsIDOMGetUserMediaSuccessCallback*, nsIDOMGetUserMediaErrorCallback*)::$_21)::Functors::Succeed(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&) + 134 at MediaUtils.h:87
> frame #14: 0x0000000103690574 XUL`void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&), void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&))::'lambda'(mozilla::dom::MediaStreamError*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&), void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&))::'lambda'(mozilla::dom::MediaStreamError*&))::Functors::Succeed(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&) [inlined] mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Resolve() + 212 at MediaUtils.h:132
> frame #15: 0x000000010369056f XUL`void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&), void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&))::'lambda'(mozilla::dom::MediaStreamError*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&), void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&))::'lambda'(mozilla::dom::MediaStreamError*&))::Functors::Succeed(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&) [inlined] mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Resolve(nsTArray<nsRefPtr<mozilla::MediaDevice> >* const&) + 40 at MediaUtils.h:111
> frame #16: 0x0000000103690547 XUL`void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&), void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&))::'lambda'(mozilla::dom::MediaStreamError*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&), void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&))::'lambda'(mozilla::dom::MediaStreamError*&))::Functors::Succeed(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&) [inlined] mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&)::operator()(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&) + 146 at MediaManager.cpp:2245
> frame #17: 0x00000001036904b5 XUL`void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(this=<unavailable>, result=<unavailable>)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&), void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&))::'lambda'(mozilla::dom::MediaStreamError*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&), void mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Then<mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&)>(mozilla::MediaManager::EnumerateDevicesImpl(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_22::operator()(nsCString const&)::'lambda'(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&))::'lambda'(mozilla::dom::MediaStreamError*&))::Functors::Succeed(nsTArray<nsRefPtr<mozilla::MediaDevice> >*&) + 21 at MediaUtils.h:87
> frame #18: 0x000000010368f4b8 XUL`mozilla::media::LambdaRunnable<mozilla::MediaManager::EnumerateRawDevices(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_19::operator()()::'lambda'()>::Run() [inlined] mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Resolve() + 136 at MediaUtils.h:132
> frame #19: 0x000000010368f4b3 XUL`mozilla::media::LambdaRunnable<mozilla::MediaManager::EnumerateRawDevices(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_19::operator()()::'lambda'()>::Run() [inlined] mozilla::media::Pledge<nsTArray<nsRefPtr<mozilla::MediaDevice> >*, mozilla::dom::MediaStreamError*>::Resolve(nsTArray<nsRefPtr<mozilla::MediaDevice> >* const&) + 33 at MediaUtils.h:111
> frame #20: 0x000000010368f492 XUL`mozilla::media::LambdaRunnable<mozilla::MediaManager::EnumerateRawDevices(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_19::operator()()::'lambda'()>::Run() [inlined] mozilla::MediaManager::EnumerateRawDevices(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_19::operator()()::'lambda'()::operator()() + 82 at MediaManager.cpp:1456
> frame #21: 0x000000010368f440 XUL`mozilla::media::LambdaRunnable<mozilla::MediaManager::EnumerateRawDevices(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_19::operator()()::'lambda'()>::Run(this=<unavailable>) + 16 at MediaUtils.h:194
> frame #22: 0x00000001015ff839 XUL`nsThread::ProcessNextEvent(this=0x000000010043d900, aMayWait=<unavailable>, aResult=0x00007fff5fbfcf77) + 1481 at nsThread.cpp:960
And on a background thread (MediaManager?) we have the release of a camera waiting for a reply from CamerasParent:
> * thread #79: tid = 0x39d5f0, 0x00007fff8fc8d136 libsystem_kernel.dylib`__psynch_cvwait + 10
> * frame #0: 0x00007fff8fc8d136 libsystem_kernel.dylib`__psynch_cvwait + 10
> frame #1: 0x00007fff9471c560 libsystem_pthread.dylib`_pthread_cond_wait + 693
> frame #2: 0x0000000101332743 libnss3.dylib`PR_WaitCondVar(cvar=0x00000001004deb00, timeout=<unavailable>) + 419 at ptsynch.c:396
> frame #3: 0x00000001016294c4 XUL`mozilla::CondVar::Wait(this=0x00000001159e85f0, aInterval=<unavailable>) + 68 at BlockingResourceBase.cpp:501
> frame #4: 0x00000001037c0e81 XUL`mozilla::camera::CamerasChild::DispatchToParent(nsIRunnable*, mozilla::MonitorAutoLock&) [inlined] mozilla::Monitor::Wait(this=<unavailable>, aInterval=4294967295) + 193 at Monitor.h:40
> frame #5: 0x00000001037c0e73 XUL`mozilla::camera::CamerasChild::DispatchToParent(nsIRunnable*, mozilla::MonitorAutoLock&) [inlined] mozilla::MonitorAutoLock::Wait(this=0x0000000135ba7988, aInterval=4294967295) + 17 at Monitor.h:88
> frame #6: 0x00000001037c0e62 XUL`mozilla::camera::CamerasChild::DispatchToParent(this=0x00000001159e8540, aRunnable=<unavailable>, aMonitor=0x0000000135ba7988) + 162 at CamerasChild.cpp:232
> frame #7: 0x00000001037c1a29 XUL`mozilla::camera::CamerasChild::ReleaseCaptureDevice(this=0x00000001159e8540, aCapEngine=<unavailable>, capture_id=<unavailable>) + 201 at CamerasChild.cpp:483
> frame #8: 0x0000000103825a71 XUL`mozilla::MediaEngineRemoteVideoSource::Deallocate(this=0x0000000115458b00) + 129 at MediaEngineRemoteVideoSource.cpp:146
> frame #9: 0x00000001036a8e64 XUL`mozilla::MediaOperationTask::Run(this=0x000000012bcbaf80) + 308 at MediaManager.cpp:315
> frame #10: 0x00000001019a52ac XUL`MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) [inlined] MessageLoop::RunTask(this=0x0000000135ba7d00, task=0x000000012bcbaf80) + 69 at message_loop.cc:364
> frame #11: 0x00000001019a5267 XUL`MessageLoop::DeferOrRunPendingTask(this=0x0000000135ba7d00, pending_task=<unavailable>) + 119 at message_loop.cc:372
> frame #12: 0x00000001019a55eb XUL`MessageLoop::DoWork(this=0x0000000135ba7d00) + 235 at message_loop.cc:459
> frame #13: 0x00000001019df3a5 XUL`mozilla::ipc::DoWorkRunnable::Run(this=<unavailable>) + 53 at MessagePump.cpp:220
> frame #14: 0x00000001015ff839 XUL`nsThread::ProcessNextEvent(this=0x0000000112f27200, aMayWait=<unavailable>, aResult=0x0000000135ba7bf7) + 1481 at nsThread.cpp:960
Flags: needinfo?(jib)
Flags: needinfo?(gpascutto)
Assignee | ||
Comment 33•9 years ago
|
||
Comment on attachment 8668794 [details]
MozReview Request: Bug 1070216 - Guard against adding a track owned by one MSG to a stream owned by another. r?padenot
Bug 1070216 - Guard against adding a track owned by one MSG to a stream owned by another. r?padenot
Assignee | ||
Comment 34•9 years ago
|
||
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot
Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot
Attachment #8668795 -
Attachment description: MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib → MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot
Attachment #8668795 -
Flags: review?(padenot)
Assignee | ||
Comment 35•9 years ago
|
||
Comment on attachment 8668796 [details]
MozReview Request: Bug 1070216 - Make it possible to disable AudioStreamAnalyser's debug canvas. r?padenot
Bug 1070216 - Make it possible to disable AudioStreamAnalyser's debug canvas. r?padenot
Assignee | ||
Comment 36•9 years ago
|
||
Comment on attachment 8668797 [details]
MozReview Request: Bug 1070216 - Let AudioStreamAnalyser accept streams with no tracks. r?padenot
Bug 1070216 - Let AudioStreamAnalyser accept streams with no tracks. r?padenot
Assignee | ||
Comment 37•9 years ago
|
||
Comment on attachment 8668798 [details]
MozReview Request: Bug 1070216 - Break out createOscillatorStream from test_gUM_addTrackRemoveTrack.html to test framework. r?padenot
Bug 1070216 - Break out createOscillatorStream from test_gUM_addTrackRemoveTrack.html to test framework. r?padenot
Assignee | ||
Comment 38•9 years ago
|
||
Comment on attachment 8668799 [details]
MozReview Request: Bug 1070216 - Test MediaStream Constructors. r?jib
Bug 1070216 - Test MediaStream Constructors. r?jib
Assignee | ||
Comment 39•9 years ago
|
||
Bug 1070216 - Guard against a null MediaInputPort in DOMMediaStream::FindPlaybackDOMTrack(). r?roc
Attachment #8668968 -
Flags: review?(roc)
Assignee | ||
Comment 40•9 years ago
|
||
Bug 1070216 - Properly manage lifetime of allocated CaptureDevices. r?jib
We currently avoid Deallocating a CaptureDevice used for multiple gUMStreams
when one of them calls Deallocate() by keeping track of how many called
Start().
The normal lifetime sequence however, is:
Allocate()
Start()
Stop()
Deallocate()
This patches fixes the lifetime management by keeping track of how many
users of the CaptureDevice called Allocate().
Attachment #8668969 -
Flags: review?(jib)
Comment 41•9 years ago
|
||
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot
r+ for the webidl
Attachment #8668795 -
Flags: review?(bugs) → review+
Comment 42•9 years ago
|
||
(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #32)
> gcp, jib, how do you suggest solving this?
> My two questions right now are,
> Shouldn't IPC stuff run off the main thread?
IPC *is* running off of the main thread here, that's not the issue. What you're seeing is that the WebRTC capture API is synchronous so CamerasChild has to block hard on responses.
> Can we fix whatever issue we had on Mac with shutdown to make sure the
> CaptureDevice is not released on main thread?
If I knew of a fix I wouldn't have put that workaround.
We might be able to make that workaround a SyncRunnable so the reply gets dispatched before anyone else has a chance to try initiating gUM, but I'm not 100% sure this works.
Anyway, why are we doing gUM on the main thread, let alone Capability querying? You can cause whole minutes of yank.
Flags: needinfo?(gpascutto)
Comment on attachment 8665902 [details]
MozReview Request: Bug 1070216 - Split DOMMediaStream::InitStreamCommon into three. r?roc
https://reviewboard.mozilla.org/r/20389/#review19087
Attachment #8665902 -
Flags: review?(roc) → review+
Comment on attachment 8668793 [details]
MozReview Request: Bug 1070216 - constify DOMMediaStream::Get[Audio/Video]Tracks(). r?roc
https://reviewboard.mozilla.org/r/21033/#review19089
Attachment #8668793 -
Flags: review?(roc) → review+
Comment on attachment 8668968 [details]
MozReview Request: Bug 1070216 - Guard against a null MediaInputPort in DOMMediaStream::FindPlaybackDOMTrack(). r?roc
https://reviewboard.mozilla.org/r/21071/#review19093
Attachment #8668968 -
Flags: review?(roc) → review+
Comment 47•9 years ago
|
||
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot
https://reviewboard.mozilla.org/r/21037/#review19133
Attachment #8668795 -
Flags: review?(padenot) → review+
Assignee | ||
Updated•9 years ago
|
Comment 48•9 years ago
|
||
Comment on attachment 8668969 [details]
MozReview Request: Bug 1070216 - Properly manage lifetime of allocated CaptureDevices. r?jib
https://reviewboard.mozilla.org/r/21073/#review19313
lgtm.
Attachment #8668969 -
Flags: review?(jib) → review+
Comment 49•9 years ago
|
||
Comment on attachment 8668969 [details]
MozReview Request: Bug 1070216 - Properly manage lifetime of allocated CaptureDevices. r?jib
https://reviewboard.mozilla.org/r/21073/#review19315
::: dom/media/webrtc/MediaEngineRemoteVideoSource.cpp:140
(Diff revision 1)
> - MonitorAutoLock lock(mMonitor);
> - empty = mSources.IsEmpty();
> - }
> + MOZ_ASSERT(mNrAllocations >= 0, "Double-deallocations are prohibited");
> +
> + if (mNrAllocations == 0) {
Hmm, one sec, the old code used a lock here, the new decrement code doesn't. Is Deallocate() called from other threads?
Attachment #8668969 -
Flags: review+
Comment 50•9 years ago
|
||
If all accesses of mNrAllocations are on the same thread then it would be nice if the code asserted that.
Flags: needinfo?(jib)
Updated•9 years ago
|
Target Milestone: --- → mozilla44
Assignee | ||
Comment 51•9 years ago
|
||
Comment on attachment 8665902 [details]
MozReview Request: Bug 1070216 - Split DOMMediaStream::InitStreamCommon into three. r?roc
Bug 1070216 - Split DOMMediaStream::InitStreamCommon into three. r?roc
This allows us to initiate a DOMMediaStream into three states:
* One that has input, owned and playback streams, for data producers
like gUM or RTCPeerConnection.
* One with owned and playback streams, for cloned DOM streams.
If a cloned DOM stream has an empty input stream connected to its
owned stream, both are regarded as not having current data.
* One with only a playback stream, for when it has been created with the
default constructor from JS. Its track set can only be changed by
addTrack() and removeTrack() which when called only affect mPlaybackStream.
Assignee | ||
Comment 52•9 years ago
|
||
Comment on attachment 8668793 [details]
MozReview Request: Bug 1070216 - constify DOMMediaStream::Get[Audio/Video]Tracks(). r?roc
Bug 1070216 - constify DOMMediaStream::Get[Audio/Video]Tracks(). r?roc
Assignee | ||
Comment 53•9 years ago
|
||
Comment on attachment 8668794 [details]
MozReview Request: Bug 1070216 - Guard against adding a track owned by one MSG to a stream owned by another. r?padenot
Bug 1070216 - Guard against adding a track owned by one MSG to a stream owned by another. r?padenot
Assignee | ||
Comment 54•9 years ago
|
||
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot
Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot
Attachment #8668795 -
Flags: review+
Assignee | ||
Comment 55•9 years ago
|
||
Comment on attachment 8668796 [details]
MozReview Request: Bug 1070216 - Make it possible to disable AudioStreamAnalyser's debug canvas. r?padenot
Bug 1070216 - Make it possible to disable AudioStreamAnalyser's debug canvas. r?padenot
Assignee | ||
Comment 56•9 years ago
|
||
Comment on attachment 8668797 [details]
MozReview Request: Bug 1070216 - Let AudioStreamAnalyser accept streams with no tracks. r?padenot
Bug 1070216 - Let AudioStreamAnalyser accept streams with no tracks. r?padenot
Assignee | ||
Comment 57•9 years ago
|
||
Comment on attachment 8668798 [details]
MozReview Request: Bug 1070216 - Break out createOscillatorStream from test_gUM_addTrackRemoveTrack.html to test framework. r?padenot
Bug 1070216 - Break out createOscillatorStream from test_gUM_addTrackRemoveTrack.html to test framework. r?padenot
Assignee | ||
Comment 58•9 years ago
|
||
Comment on attachment 8668799 [details]
MozReview Request: Bug 1070216 - Test MediaStream Constructors. r?jib
Bug 1070216 - Test MediaStream Constructors. r?jib
Assignee | ||
Comment 59•9 years ago
|
||
Comment on attachment 8668968 [details]
MozReview Request: Bug 1070216 - Guard against a null MediaInputPort in DOMMediaStream::FindPlaybackDOMTrack(). r?roc
Bug 1070216 - Guard against a null MediaInputPort in DOMMediaStream::FindPlaybackDOMTrack(). r?roc
Assignee | ||
Comment 60•9 years ago
|
||
Comment on attachment 8668969 [details]
MozReview Request: Bug 1070216 - Properly manage lifetime of allocated CaptureDevices. r?jib
Bug 1070216 - Properly manage lifetime of allocated CaptureDevices. r?jib
We currently avoid Deallocating a CaptureDevice used for multiple gUMStreams
when one of them calls Deallocate() by keeping track of how many called
Start().
The normal lifetime sequence however, is:
Allocate()
Start()
Stop()
Deallocate()
This patches fixes the lifetime management by keeping track of how many
users of the CaptureDevice called Allocate().
Attachment #8668969 -
Flags: review?(jib)
Assignee | ||
Comment 61•9 years ago
|
||
Bug 1070216 - Assert main MediaEngine APIs are called on the owning thread. r?jib
Attachment #8673449 -
Flags: review?(jib)
Assignee | ||
Comment 62•9 years ago
|
||
Comment 63•9 years ago
|
||
Comment on attachment 8673449 [details]
MozReview Request: Bug 1070216 - Assert main MediaEngine APIs are called on the owning thread. r?jib
https://reviewboard.mozilla.org/r/21925/#review19707
::: dom/media/webrtc/MediaEngine.h:13
(Diff revision 1)
> +#include "nsPrintfCString.h"
litter?
Attachment #8673449 -
Flags: review?(jib) → review+
Comment 64•9 years ago
|
||
Comment on attachment 8668969 [details]
MozReview Request: Bug 1070216 - Properly manage lifetime of allocated CaptureDevices. r?jib
https://reviewboard.mozilla.org/r/21073/#review19711
No change (bug 1169360)
Attachment #8668969 -
Flags: review?(jib) → review+
Assignee | ||
Comment 65•9 years ago
|
||
(In reply to Jan-Ivar Bruaroey [:jib] from comment #64)
> Comment on attachment 8668969 [details]
> MozReview Request: Bug 1070216 - Properly manage lifetime of allocated
> CaptureDevices. r?jib
>
> https://reviewboard.mozilla.org/r/21073/#review19711
>
> No change (bug 1169360)
Well, you unshipped it before :-)
Assignee | ||
Comment 66•9 years ago
|
||
(In reply to Jan-Ivar Bruaroey [:jib] from comment #63)
> Comment on attachment 8673449 [details]
> MozReview Request: Bug 1070216 - Assert main MediaEngine APIs are called on
> the owning thread. r?jib
>
> https://reviewboard.mozilla.org/r/21925/#review19707
>
> ::: dom/media/webrtc/MediaEngine.h:13
> (Diff revision 1)
> > +#include "nsPrintfCString.h"
>
> litter?
Seems so indeed, I think I misinterpreted a compilation failure and added it then.
Comment 67•9 years ago
|
||
(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #65)
> Well, you unshipped it before :-)
Whoops, you're right. :*) Looks like commenting in mozReview automatically does that unless I remember to uncheck "open an issue".
Comment 68•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/785c3808c665
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c7d2c93ab42
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e158843f39c
https://hg.mozilla.org/integration/mozilla-inbound/rev/66c0ffdaac94
https://hg.mozilla.org/integration/mozilla-inbound/rev/e01e6e1bf048
https://hg.mozilla.org/integration/mozilla-inbound/rev/39ea089dda2b
https://hg.mozilla.org/integration/mozilla-inbound/rev/74acdb4ee6b2
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c2e1e740ef1
https://hg.mozilla.org/integration/mozilla-inbound/rev/f93f6f5b57e9
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f4ae4d923bc
https://hg.mozilla.org/integration/mozilla-inbound/rev/075de7ffc645
Comment 69•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/785c3808c665
https://hg.mozilla.org/mozilla-central/rev/6c7d2c93ab42
https://hg.mozilla.org/mozilla-central/rev/0e158843f39c
https://hg.mozilla.org/mozilla-central/rev/66c0ffdaac94
https://hg.mozilla.org/mozilla-central/rev/e01e6e1bf048
https://hg.mozilla.org/mozilla-central/rev/39ea089dda2b
https://hg.mozilla.org/mozilla-central/rev/74acdb4ee6b2
https://hg.mozilla.org/mozilla-central/rev/3c2e1e740ef1
https://hg.mozilla.org/mozilla-central/rev/f93f6f5b57e9
https://hg.mozilla.org/mozilla-central/rev/2f4ae4d923bc
https://hg.mozilla.org/mozilla-central/rev/075de7ffc645
You need to log in
before you can comment on or make changes to this bug.
Description
•