Clone MediaStream video tracks correctly with spec-compliant constraints.

content/renderer/media/media_stream_center.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/media_stream_center.h"
 
 #include <stddef.h>
 
 #include <string>
 
@@ skipping 65 matching lines @@
     // cloned track and not the originating source.
     // Also - source.constraints() returns an uninitialized constraint if the
     // source is coming from a remote video track. See http://crbug/287805.
     blink::WebMediaConstraints constraints = source.constraints();
     if (constraints.isNull())
       constraints.initialize();
     writable_track.setTrackData(new MediaStreamVideoTrack(
         native_source, constraints,
         MediaStreamVideoSource::ConstraintsCallback(), track.isEnabled()));
   } else {
-    // TODO(guidou): All existing uses are without constraints except for the
-    // case of cloning a track. To fix this, create a new function in
-    // MediaStreamCenter specifically for cloning.
     writable_track.setTrackData(new MediaStreamVideoTrack(
         native_source, MediaStreamVideoSource::ConstraintsCallback(),
         track.isEnabled()));
   }
 }
 
+void CloneNativeVideoMediaStreamTrack(
+    const blink::WebMediaStreamTrack& original,
+    const blink::WebMediaStreamTrack& clone) {
+  DCHECK(!clone.getTrackData());
+  blink::WebMediaStreamSource source = clone.source();
+  DCHECK_EQ(source.getType(), blink::WebMediaStreamSource::TypeVideo);
+  MediaStreamVideoSource* native_source =
+      MediaStreamVideoSource::GetVideoSource(source);
+  DCHECK(native_source);
+  blink::WebMediaStreamTrack writable_clone(clone);

[hbos_chromium, 2017/04/04 11:21:43]

Can you add a comment explaining that doing setTrackData on the "writable" sets
the track data of the original |clone|'s MediaStreamComponent?
Same comment should be added to CreateNativeVideoMediaStreamTrack.

Alternatively, does CloneNativeVideoMediaStreamTrack have to take a const
argument (blink::MediaStreamCenter has non-const ptrs)? This seems like a way of
avoiding essentially a const_cast since a const-WebMediaStreamTrack leaks a
non-const TrackData pointer, an odd thing to do, but existing behavior.

[Guido Urdaneta, 2017/04/04 15:42:53]

Went instead for passing by value. WebMSTrack just wraps a pointer to
MSComponent, so it's OK to pass by value.

+  if (IsOldVideoConstraints()) {
+    // TODO(perkj): The constraints to use here should be passed from blink when
+    // a new track is created. For cloning, it should be the constraints of the
+    // cloned track and not the originating source.
+    // Also - source.constraints() returns an uninitialized constraint if the
+    // source is coming from a remote video track. See http://crbug/287805.
+    blink::WebMediaConstraints constraints = source.constraints();
+    if (constraints.isNull())
+      constraints.initialize();
+    writable_clone.setTrackData(new MediaStreamVideoTrack(
+        native_source, constraints,
+        MediaStreamVideoSource::ConstraintsCallback(), clone.isEnabled()));
+  } else {
+    MediaStreamVideoTrack* original_track =
+        MediaStreamVideoTrack::GetVideoTrack(original);
+    DCHECK(original_track);
+    writable_clone.setTrackData(new MediaStreamVideoTrack(
+        native_source, original_track->adapter_settings(),
+        original_track->noise_reduction(), original_track->is_screencast(),
+        original_track->min_frame_rate(),
+        MediaStreamVideoSource::ConstraintsCallback(), clone.isEnabled()));
+  }
+}
+
 }  // namespace
 
 MediaStreamCenter::MediaStreamCenter(
     blink::WebMediaStreamCenterClient* client,
     PeerConnectionDependencyFactory* factory) {}
 
 MediaStreamCenter::~MediaStreamCenter() {}
 
 void MediaStreamCenter::didCreateMediaStreamTrack(
     const blink::WebMediaStreamTrack& track) {
   DVLOG(1) << "MediaStreamCenter::didCreateMediaStreamTrack";
   DCHECK(!track.isNull() && !track.getTrackData());
   DCHECK(!track.source().isNull());
 
   switch (track.source().getType()) {
     case blink::WebMediaStreamSource::TypeAudio:
       CreateNativeAudioMediaStreamTrack(track);
       break;
     case blink::WebMediaStreamSource::TypeVideo:
       CreateNativeVideoMediaStreamTrack(track);
       break;
   }
 }
 
+void MediaStreamCenter::didCloneMediaStreamTrack(
+    const blink::WebMediaStreamTrack& original,
+    const blink::WebMediaStreamTrack& clone) {
+  DCHECK(!clone.isNull());
+  DCHECK(!clone.getTrackData());
+  DCHECK(!clone.source().isNull());
+
+  switch (clone.source().getType()) {
+    case blink::WebMediaStreamSource::TypeAudio:
+      CreateNativeAudioMediaStreamTrack(clone);
+      break;
+    case blink::WebMediaStreamSource::TypeVideo:
+      CloneNativeVideoMediaStreamTrack(original, clone);
+      break;
+  }
+}
+
 void MediaStreamCenter::didSetContentHint(
     const blink::WebMediaStreamTrack& track) {
   MediaStreamTrack* native_track = MediaStreamTrack::GetTrack(track);
   if (native_track)
     native_track->SetContentHint(track.contentHint());
 }
 
 void MediaStreamCenter::didEnableMediaStreamTrack(
     const blink::WebMediaStreamTrack& track) {
   MediaStreamTrack* native_track =
@@ skipping 77 matching lines @@
 
 bool MediaStreamCenter::didRemoveMediaStreamTrack(
     const blink::WebMediaStream& stream,
     const blink::WebMediaStreamTrack& track) {
   DVLOG(1) << "MediaStreamCenter::didRemoveMediaStreamTrack";
   MediaStream* native_stream = MediaStream::GetMediaStream(stream);
   return native_stream->RemoveTrack(track);
 }
 
 }  // namespace content