Bug 146313

Summary: WebRTC: Update the MediaStream API
Product: WebKit Reporter: Adam Bergkvist <adam.bergkvist>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, jonlee, mdaiter, pnormand
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 143211    
Attachments:
Description Flags
Proposed patch
eric.carlson: review+
Patch for landing
none
Patch for landing none

Description Adam Bergkvist 2015-06-25 05:30:03 PDT
Update the MediaStream API to match the latest Media Capture and Streams specification [1].

In WebKit, the MediaStream API is mainly implemented by MediaStream and MediaStreamTrack with their corresponding platform representations, and RealtimeMediaSource.

[1] http://w3c.github.io/mediacapture-main/archives/20150523/getusermedia.html
Comment 1 Adam Bergkvist 2015-06-28 07:09:29 PDT
Created attachment 255723 [details]
Proposed patch

Proposed patch
Comment 2 Eric Carlson 2015-06-28 10:45:58 PDT
Comment on attachment 255723 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255723&action=review

> Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp:90
> +    Vector<RefPtr<MediaStreamTrackPrivate>> tracks;
> +    tracks.reserveCapacity(m_trackSet.size());
>  
> -    unsigned providedTracksSize = audioPrivateTracks.size() + videoPrivateTracks.size();
> -    unsigned tracksSize = m_audioPrivateTracks.size() + m_videoPrivateTracks.size();
> +    for (auto& track : m_trackSet.values())
> +        tracks.append(track);
>  
> -    if (providedTracksSize > 0 && tracksSize > 0)
> -        m_isActive = true;
> +    return tracks;

Nit: you could use copyValuesToVector here.

> Source/WebCore/platform/mediastream/MediaStreamPrivate.h:65
> +    enum NotifyClientOption { NotifyClient, DontNotifyClient };

This should be "enum class".
Comment 3 Adam Bergkvist 2015-06-29 07:25:51 PDT
(In reply to comment #2)
> Comment on attachment 255723 [details]
> Proposed patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=255723&action=review
> 
> > Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp:90
> > +    Vector<RefPtr<MediaStreamTrackPrivate>> tracks;
> > +    tracks.reserveCapacity(m_trackSet.size());
> >  
> > -    unsigned providedTracksSize = audioPrivateTracks.size() + videoPrivateTracks.size();
> > -    unsigned tracksSize = m_audioPrivateTracks.size() + m_videoPrivateTracks.size();
> > +    for (auto& track : m_trackSet.values())
> > +        tracks.append(track);
> >  
> > -    if (providedTracksSize > 0 && tracksSize > 0)
> > -        m_isActive = true;
> > +    return tracks;
> 
> Nit: you could use copyValuesToVector here.

Fixed.

> > Source/WebCore/platform/mediastream/MediaStreamPrivate.h:65
> > +    enum NotifyClientOption { NotifyClient, DontNotifyClient };
> 
> This should be "enum class".

Fixed (fixed other enum as well)
Comment 4 Adam Bergkvist 2015-06-29 07:27:06 PDT
Created attachment 255747 [details]
Patch for landing
Comment 5 Adam Bergkvist 2015-06-29 07:28:10 PDT
Thanks Eric for reviewing. I'll let the EWS complete before setting cq+.
Comment 6 Adam Bergkvist 2015-06-29 11:39:10 PDT
Created attachment 255764 [details]
Patch for landing

With reviewer name added
Comment 7 WebKit Commit Bot 2015-06-29 12:54:46 PDT
Comment on attachment 255764 [details]
Patch for landing

Clearing flags on attachment: 255764

Committed r186081: <http://trac.webkit.org/changeset/186081>
Comment 8 WebKit Commit Bot 2015-06-29 12:54:49 PDT
All reviewed patches have been landed.  Closing bug.