1041493 - add support for "browser" to mediaSource constraint

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: browser_mediaSource.patch

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8459537 [details] [diff] [review]
browser_mediaSource.patch

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

r+ as a patch so far as it goes.  Larger question is what we're using this for; what it does on desktop, what's the UI bug for this; mods to MediaManager.cpp to provide info about "browser" being in-use for UI purposes, etc.  So this is only part of the overall solution it seems.  There should be a UI/UX bug(s) linked to this as well, plus probably security discussion.

::: dom/webidl/Constraints.webidl
@@ +18,5 @@
>      "camera",
>      "screen",
>      "application",
> +    "window",
> +    "browser"

DOM peer needs to r+ this

::: dom/webidl/MediaTrackConstraintSet.webidl
@@ +38,4 @@
>  
>  // TODO: Bug 767924 sequences in unions
>  //typedef (VideoFacingModeEnum or sequence<VideoFacingModeEnum>) ConstrainVideoFacingMode;
> +//typedef (MediaSourceEnum or sequence<MediaSourceEnum>) ConstrainMediaSource;

Did this change?

::: media/webrtc/trunk/webrtc/video_engine/browser_capture_impl.h
@@ +11,5 @@
> +  public:
> +    virtual uint32_t NumberOfDevices() { return 1; }
> +
> +    virtual int32_t GetDeviceName(
> +                                  uint32_t deviceNumber,

Remove spurious line break (here and below) - is there some editor that encourages this format?

@@ +16,5 @@
> +                                  char* deviceNameUTF8,
> +                                  uint32_t deviceNameLength,
> +                                  char* deviceUniqueIdUTF8,
> +                                  uint32_t deviceUniqueIdUTF8Length,
> +                                  char* productUniqueIdUTF8 = 0,

NULL

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Randell Jesup [:jesup] from comment #1)
> Comment on attachment 8459537 [details] [diff] [review]
> browser_mediaSource.patch
> 
> Review of attachment 8459537 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ as a patch so far as it goes.  Larger question is what we're using this
> for; what it does on desktop, what's the UI bug for this; mods to
> MediaManager.cpp to provide info about "browser" being in-use for UI
> purposes, etc.  So this is only part of the overall solution it seems. 
> There should be a UI/UX bug(s) linked to this as well, plus probably
> security discussion.
The feature driving this is tab mirroring, which I'm working on in bug 1037015.


> 
> ::: media/webrtc/trunk/webrtc/video_engine/browser_capture_impl.h
> @@ +11,5 @@
> > +  public:
> > +    virtual uint32_t NumberOfDevices() { return 1; }
> > +
> > +    virtual int32_t GetDeviceName(
> > +                                  uint32_t deviceNumber,
> 
> Remove spurious line break (here and below) - is there some editor that
> encourages this format?

No. I can't really explain these really.

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8459537 [details] [diff] [review]
browser_mediaSource.patch

r?khuey for the webidl change

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

this landed http://hg.mozilla.org/mozilla-central/rev/3a53a1cc3dbf