Clone MediaStream video tracks correctly with spec-compliant constraints.

Clone MediaStream video tracks correctly with spec-compliant constraints.

The old algorithm copied constraints to the source, and used them for
cloning.
With the new algorithm, sources know about source settings and tracks
about track settings. This CL copies settings from the original track
to the clone.

BUG=657733
Review-Url: https://codereview.chromium.org/2791303002
Cr-Commit-Position: refs/heads/master@{#462424}
Committed: https://chromium.googlesource.com/chromium/src/+/501bc50c7265030e4873ce16b3e583a50fd5b558

[Guido Urdaneta, 2017-04-03 16:47:48]

Description was changed from

==========
Clone MediaStream video tracks correctly with spec-compliant constraints.

The old algorithm copied constraints to the source, and used them for
cloning.
With the new algorithm, sources know about source settings and tracks
about track settings. This CL copies settings from the original track
to the clone.

BUG=657733
==========

to

==========
Clone MediaStream video tracks correctly with spec-compliant constraints.

The old algorithm copied constraints to the source, and used them for
cloning.
With the new algorithm, sources know about source settings and tracks
about track settings. This CL copies settings from the original track
to the clone.

BUG=657733
==========

[Guido Urdaneta, 2017-04-03 16:47:48]

guidou@chromium.org changed reviewers:
+ hbos@chromium.org

[Guido Urdaneta, 2017-04-03 16:50:08]

guidou@chromium.org changed reviewers:
+ tkent@chromium.org

[Guido Urdaneta, 2017-04-03 16:50:09]

hbos@chromium.org: Please review changes in content/ and modules/mediastream
tkent@chromium.org: Please review changes in platform/mediastream and
public/platform

[Guido Urdaneta, 2017-04-03 16:50:44]

The CQ bit was checked by guidou@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-03 16:51:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 17:20:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-03 17:20:56]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Guido Urdaneta, 2017-04-03 18:25:33]

The CQ bit was checked by guidou@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-03 18:26:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 20:53:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-03 20:53:12]

Dry run: This issue passed the CQ dry run.

[tkent, 2017-04-04 02:16:04]

> tkent@chromium.org: Please review changes in platform/mediastream and
public/platform

lgtm

[hbos_chromium, 2017-04-04 11:21:44]

lgtm

https://codereview.chromium.org/2791303002/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_center.cc (right):

https://codereview.chromium.org/2791303002/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_center.cc:101: blink::WebMediaStreamTrack
writable_clone(clone);
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]

https://codereview.chromium.org/2791303002/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_center.cc (right):

https://codereview.chromium.org/2791303002/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_center.cc:101: blink::WebMediaStreamTrack
writable_clone(clone);
On 2017/04/04 11:21:43, hbos_chromium wrote:
> 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.

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

[Guido Urdaneta, 2017-04-05 17:14:14]

The CQ bit was checked by guidou@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-05 17:14:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 17:19:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-05 17:19:10]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Guido Urdaneta, 2017-04-05 17:26:52]

Patchset #4 (id:60001) has been deleted

[Guido Urdaneta, 2017-04-05 17:26:59]

The CQ bit was checked by guidou@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-05 17:27:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 20:17:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-05 20:17:04]

Dry run: This issue passed the CQ dry run.

[Guido Urdaneta, 2017-04-06 11:20:48]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2017-04-06 11:20:49]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
hbos@chromium.org
Link to the patchset: https://codereview.chromium.org/2791303002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-06 11:21:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-06 11:27:32]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1491477648815080,
"parent_rev": "96ea303ff0f2ed2bc7fa35387c0f1b85b1fc0e40", "commit_rev":
"501bc50c7265030e4873ce16b3e583a50fd5b558"}

[commit-bot: I haz the power, 2017-04-06 11:28:31]

Description was changed from

==========
Clone MediaStream video tracks correctly with spec-compliant constraints.

The old algorithm copied constraints to the source, and used them for
cloning.
With the new algorithm, sources know about source settings and tracks
about track settings. This CL copies settings from the original track
to the clone.

BUG=657733
==========

to

==========
Clone MediaStream video tracks correctly with spec-compliant constraints.

The old algorithm copied constraints to the source, and used them for
cloning.
With the new algorithm, sources know about source settings and tracks
about track settings. This CL copies settings from the original track
to the clone.

BUG=657733

Review-Url: https://codereview.chromium.org/2791303002
Cr-Commit-Position: refs/heads/master@{#462424}
Committed:
https://chromium.googlesource.com/chromium/src/+/501bc50c7265030e4873ce16b3e5...
==========

[commit-bot: I haz the power, 2017-04-06 11:28:32]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/501bc50c7265030e4873ce16b3e5...