Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(103)

Issue 2727583007: HTMLMediaElement capture: teach the captured MStream to follow up source events (Closed)

Created:
3 years, 9 months ago by mcasas
Modified:
3 years, 9 months ago
CC:
chromium-reviews, haraken, blink-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

HTMLMediaElement capture: teach the captured MStream to follow up source events wip This CL adds support for the MediaStream created by calling captureStream() on a <video/audio> to follow the events of the source <video/audio>, namely: - if the source tag ends, the captured MediaStreamTracks are ended and the captured MediaStream is ended (which wasn't the case before). - if the source tag hits an onloadedmetadata, MediaStreamTracks are added to the captured MediaStream as fit. -- this also allows for a MediaStream-with-no-tracks to be created out of a source tag with no content. Added/beefed up LayoutTests for this. MediaStream::{add,remove}RemoteTrack are renamed to {add,remove}TrackByComponent. The reason for the original name is that only internal add/removal of tracks should fire an on{add/remove}track [1], and until this CL this was only possible for remote tracks. [1] https://developer.mozilla.org/en-US/docs/Web/Events/addtrack BUG=698514 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2727583007 Cr-Commit-Position: refs/heads/master@{#454964} Committed: https://chromium.googlesource.com/chromium/src/+/d80e6732a30c9f8b5c725fc98b013577799022cd

Patch Set 1 #

Patch Set 2 : Connected on{add/remove}track events, moar LayoutTests #

Total comments: 3

Patch Set 3 : Made the media element and the MediaStream Members of MediaElementEventListener #

Total comments: 8

Patch Set 4 : haraken@ comments #

Messages

Total messages: 41 (29 generated)
mcasas
emircan@ PTAL
3 years, 9 months ago (2017-03-04 22:53:17 UTC) #17
haraken
https://codereview.chromium.org/2727583007/diff/120001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp File third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp (right): https://codereview.chromium.org/2727583007/diff/120001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp#newcode46 third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp:46: WeakMember<MediaStream> m_mediaStream; Why do you want to make these ...
3 years, 9 months ago (2017-03-05 07:04:18 UTC) #19
mcasas
https://codereview.chromium.org/2727583007/diff/120001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp File third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp (right): https://codereview.chromium.org/2727583007/diff/120001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp#newcode46 third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp:46: WeakMember<MediaStream> m_mediaStream; On 2017/03/05 07:04:18, haraken wrote: > > ...
3 years, 9 months ago (2017-03-05 18:34:01 UTC) #20
haraken
https://codereview.chromium.org/2727583007/diff/120001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp File third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp (right): https://codereview.chromium.org/2727583007/diff/120001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp#newcode46 third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp:46: WeakMember<MediaStream> m_mediaStream; On 2017/03/05 18:34:01, mcasas wrote: > On ...
3 years, 9 months ago (2017-03-06 01:29:38 UTC) #21
mcasas
On 2017/03/06 01:29:38, haraken wrote: > https://codereview.chromium.org/2727583007/diff/120001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp > File > third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp > (right): > > ...
3 years, 9 months ago (2017-03-06 02:58:12 UTC) #22
mcasas
guidou@ PTAL at the rename in MediaStream* files haraken@ would you RS this patch or ...
3 years, 9 months ago (2017-03-06 18:10:32 UTC) #28
haraken
In terms of the object lifetime, LGTM. https://codereview.chromium.org/2727583007/diff/140001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp File third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp (right): https://codereview.chromium.org/2727583007/diff/140001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp#newcode56 third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp:56: for (size_t ...
3 years, 9 months ago (2017-03-06 18:37:29 UTC) #29
Guido Urdaneta
Source/modules/mediastream lgtm
3 years, 9 months ago (2017-03-06 19:30:11 UTC) #30
mcasas
https://codereview.chromium.org/2727583007/diff/140001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp File third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp (right): https://codereview.chromium.org/2727583007/diff/140001/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp#newcode56 third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp:56: for (size_t i = 0; i < tracks.size(); ++i) ...
3 years, 9 months ago (2017-03-06 19:38:59 UTC) #33
emircan
lgtm
3 years, 9 months ago (2017-03-06 19:42:29 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2727583007/160001
3 years, 9 months ago (2017-03-06 19:54:43 UTC) #38
commit-bot: I haz the power
3 years, 9 months ago (2017-03-06 21:36:19 UTC) #41
Message was sent while issue was closed.
Committed patchset #4 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/d80e6732a30c9f8b5c725fc98b01...

Powered by Google App Engine
This is Rietveld 408576698