Last Comment Bug 1070216 - Add support for MediaStream constructors (from MediaStreamTracks)
: Add support for MediaStream constructors (from MediaStreamTracks)
Status: RESOLVED FIXED
: dev-doc-needed
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Other Branch
: All All
P2 normal with 3 votes (vote)
: mozilla44
Assigned To: Andreas Pehrson [:pehrsons] (Telenor)
:
: Maire Reavy [:mreavy] Please needinfo me
Mentors:
: 758391 (view as bug list)
Depends on: 882145 1103188 1210852
Blocks: 910249 985265 foxeye 910215 952369 1045810
  Show dependency treegraph
 
Reported: 2014-09-19 15:14 PDT by Randell Jesup [:jesup]
Modified: 2015-11-11 00:40 PST (History)
26 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
MozReview Request: Bug 1070216 - Split DOMMediaStream::InitStreamCommon into three. r?roc (40 bytes, text/x-review-board-request)
2015-09-25 03:47 PDT, Andreas Pehrson [:pehrsons] (Telenor)
roc: review+
Details | Review
MozReview Request: Bug 1070216 - constify DOMMediaStream::Get[Audio/Video]Tracks(). r?roc (40 bytes, text/x-review-board-request)
2015-10-01 21:36 PDT, Andreas Pehrson [:pehrsons] (Telenor)
roc: review+
Details | Review
MozReview Request: Bug 1070216 - Guard against adding a track owned by one MSG to a stream owned by another. r?padenot (40 bytes, text/x-review-board-request)
2015-10-01 21:36 PDT, Andreas Pehrson [:pehrsons] (Telenor)
padenot: review+
Details | Review
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot (40 bytes, text/x-review-board-request)
2015-10-01 21:36 PDT, Andreas Pehrson [:pehrsons] (Telenor)
jib: review+
padenot: review+
Details | Review
MozReview Request: Bug 1070216 - Make it possible to disable AudioStreamAnalyser's debug canvas. r?padenot (40 bytes, text/x-review-board-request)
2015-10-01 21:36 PDT, Andreas Pehrson [:pehrsons] (Telenor)
padenot: review+
Details | Review
MozReview Request: Bug 1070216 - Let AudioStreamAnalyser accept streams with no tracks. r?padenot (40 bytes, text/x-review-board-request)
2015-10-01 21:36 PDT, Andreas Pehrson [:pehrsons] (Telenor)
padenot: review+
Details | Review
MozReview Request: Bug 1070216 - Break out createOscillatorStream from test_gUM_addTrackRemoveTrack.html to test framework. r?padenot (40 bytes, text/x-review-board-request)
2015-10-01 21:36 PDT, Andreas Pehrson [:pehrsons] (Telenor)
padenot: review+
Details | Review
MozReview Request: Bug 1070216 - Test MediaStream Constructors. r?jib (40 bytes, text/x-review-board-request)
2015-10-01 21:36 PDT, Andreas Pehrson [:pehrsons] (Telenor)
jib: review+
Details | Review
MozReview Request: Bug 1070216 - Guard against a null MediaInputPort in DOMMediaStream::FindPlaybackDOMTrack(). r?roc (40 bytes, text/x-review-board-request)
2015-10-02 08:04 PDT, Andreas Pehrson [:pehrsons] (Telenor)
roc: review+
Details | Review
MozReview Request: Bug 1070216 - Properly manage lifetime of allocated CaptureDevices. r?jib (40 bytes, text/x-review-board-request)
2015-10-02 08:04 PDT, Andreas Pehrson [:pehrsons] (Telenor)
jib: review+
Details | Review
MozReview Request: Bug 1070216 - Assert main MediaEngine APIs are called on the owning thread. r?jib (40 bytes, text/x-review-board-request)
2015-10-13 20:30 PDT, Andreas Pehrson [:pehrsons] (Telenor)
jib: review+
Details | Review

Description User image Randell Jesup [:jesup] 2014-09-19 15:14:09 PDT
+++ 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 User image Iñaki Baz Castillo 2015-01-12 05:01:02 PST
Unfeasible if MediaStream.addTrack() is not implemented: #1103188
Comment 2 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-05-18 19:00:19 PDT
This should be simple with a default constructor and addTrack() (blocking this bug).
Comment 3 User image Paul Adenot (:padenot) 2015-05-19 00:49:20 PDT
This will be very useful for bug 1156472, see comment 1. In fact, it's kind of blocking it.
Comment 4 User image Jan-Ivar Bruaroey [:jib] 2015-05-27 19:05:32 PDT
*** Bug 758391 has been marked as a duplicate of this bug. ***
Comment 5 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-09-25 03:47:13 PDT
Created attachment 8665902 [details]
MozReview Request: Bug 1070216 - Split DOMMediaStream::InitStreamCommon into three. r?roc

Bug 1070216 - Implement MediaStream Constructors. r?smaug,roc
Comment 6 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-09-25 03:53:34 PDT
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()?
Comment 7 User image Paul Adenot (:padenot) 2015-09-25 06:16:36 PDT
(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.
Comment 8 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 21:36:05 PDT
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.
Comment 9 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 21:36:07 PDT
Created 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
Comment 10 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 21:36:10 PDT
Created 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
Comment 11 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 21:36:12 PDT
Created attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot

Bug 1070216 - Implement MediaStream constructors. r?smaug,jib
Comment 12 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 21:36:15 PDT
Created 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
Comment 13 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 21:36:17 PDT
Created 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
Comment 14 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 21:36:20 PDT
Created 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
Comment 15 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 21:36:22 PDT
Created attachment 8668799 [details]
MozReview Request: Bug 1070216 - Test MediaStream Constructors. r?jib

Bug 1070216 - Test MediaStream Constructors. r?jib
Comment 16 User image Jan-Ivar Bruaroey [:jib] 2015-10-01 22:20:08 PDT
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.
Comment 17 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 22:30:07 PDT
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..
Comment 18 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 22:30:41 PDT
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot

Bug 1070216 - Implement MediaStream constructors. r?smaug,jib
Comment 19 User image Jan-Ivar Bruaroey [:jib] 2015-10-01 22:33:54 PDT
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.
Comment 20 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 22:39:31 PDT
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 User image Jan-Ivar Bruaroey [:jib] 2015-10-01 22:58:35 PDT
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 User image Jan-Ivar Bruaroey [:jib] 2015-10-01 23:00:02 PDT
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
Comment 23 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-01 23:02:27 PDT
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 :-)
Comment 24 User image Paul Adenot (:padenot) 2015-10-02 01:14:17 PDT
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 User image Paul Adenot (:padenot) 2015-10-02 01:14:35 PDT
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot

https://reviewboard.mozilla.org/r/21037/#review18943
Comment 26 User image Paul Adenot (:padenot) 2015-10-02 01:14:48 PDT
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 User image Paul Adenot (:padenot) 2015-10-02 01:15:35 PDT
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
Comment 28 User image Paul Adenot (:padenot) 2015-10-02 01:16:17 PDT
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
Comment 29 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 01:18:19 PDT
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 User image Paul Adenot (:padenot) 2015-10-02 01:24:36 PDT
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.
Comment 31 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 03:30:54 PDT
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.
Comment 32 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 07:15:56 PDT
(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
Comment 33 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 08:04:23 PDT
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
Comment 34 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 08:04:26 PDT
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
Comment 35 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 08:04:28 PDT
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
Comment 36 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 08:04:31 PDT
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
Comment 37 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 08:04:33 PDT
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
Comment 38 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 08:04:35 PDT
Comment on attachment 8668799 [details]
MozReview Request: Bug 1070216 - Test MediaStream Constructors. r?jib

Bug 1070216 - Test MediaStream Constructors. r?jib
Comment 39 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 08:04:37 PDT
Created 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
Comment 40 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-02 08:04:41 PDT
Created 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().
Comment 41 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2015-10-02 08:21:05 PDT
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot

r+ for the webidl
Comment 42 User image Gian-Carlo Pascutto [:gcp] 2015-10-02 08:58:48 PDT
(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.
Comment 43 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2015-10-05 02:14:33 PDT
Comment on attachment 8665902 [details]
MozReview Request: Bug 1070216 - Split DOMMediaStream::InitStreamCommon into three. r?roc

https://reviewboard.mozilla.org/r/20389/#review19087
Comment 44 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2015-10-05 02:14:44 PDT
Comment on attachment 8668793 [details]
MozReview Request: Bug 1070216 - constify DOMMediaStream::Get[Audio/Video]Tracks(). r?roc

https://reviewboard.mozilla.org/r/21033/#review19089
Comment 45 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2015-10-05 02:14:48 PDT
https://reviewboard.mozilla.org/r/21033/#review19091
Comment 46 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2015-10-05 02:15:08 PDT
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
Comment 47 User image Paul Adenot (:padenot) 2015-10-05 08:33:10 PDT
Comment on attachment 8668795 [details]
MozReview Request: Bug 1070216 - Implement MediaStream constructors. r?smaug,jib,padenot

https://reviewboard.mozilla.org/r/21037/#review19133
Comment 48 User image Jan-Ivar Bruaroey [:jib] 2015-10-07 06:32:02 PDT
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.
Comment 49 User image Jan-Ivar Bruaroey [:jib] 2015-10-07 06:36:31 PDT
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?
Comment 50 User image Jan-Ivar Bruaroey [:jib] 2015-10-07 07:16:59 PDT
If all accesses of mNrAllocations are on the same thread then it would be nice if the code asserted that.
Comment 51 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:07 PDT
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.
Comment 52 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:09 PDT
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
Comment 53 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:11 PDT
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
Comment 54 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:14 PDT
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
Comment 55 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:16 PDT
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
Comment 56 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:19 PDT
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
Comment 57 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:21 PDT
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
Comment 58 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:23 PDT
Comment on attachment 8668799 [details]
MozReview Request: Bug 1070216 - Test MediaStream Constructors. r?jib

Bug 1070216 - Test MediaStream Constructors. r?jib
Comment 59 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:25 PDT
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
Comment 60 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:28 PDT
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().
Comment 61 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:30:31 PDT
Created attachment 8673449 [details]
MozReview Request: Bug 1070216 - Assert main MediaEngine APIs are called on the owning thread. r?jib

Bug 1070216 - Assert main MediaEngine APIs are called on the owning thread. r?jib
Comment 62 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-13 20:36:22 PDT
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=853af28af830
Comment 63 User image Jan-Ivar Bruaroey [:jib] 2015-10-14 07:15:24 PDT
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?
Comment 64 User image Jan-Ivar Bruaroey [:jib] 2015-10-14 07:20:54 PDT
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)
Comment 65 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-14 09:56:00 PDT
(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 :-)
Comment 66 User image Andreas Pehrson [:pehrsons] (Telenor) 2015-10-14 10:07:07 PDT
(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 User image Jan-Ivar Bruaroey [:jib] 2015-10-14 10:17:34 PDT
(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".

Note You need to log in before you can comment on or make changes to this bug.