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

Issue 12902002: Remove WebVideoFrame, WebVideoFrameProvider, and WebVideoLayer. (Closed)

Created:
7 years, 9 months ago by danakj
Modified:
7 years, 9 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org, cc-bugs_chromium.org, darin-cc_chromium.org, piman, backer, wjia(left Chromium)
Visibility:
Public.

Description

Remove WebVideoFrame, WebVideoFrameProvider, and WebVideoLayer. These class are no longer used. Removes the #ifdefs guarding the old unused paths, and removes the WebVideo* files entirely. R=jamesr BUG=196810 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190706

Patch Set 1 #

Patch Set 2 : #

Total comments: 8

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : #

Patch Set 9 : Android Compiles #

Patch Set 10 : nits #

Patch Set 11 : Add some locks back #

Patch Set 12 : movedownership #

Patch Set 13 : got MS and Android too #

Patch Set 14 : #

Patch Set 15 : #

Patch Set 16 : No provider client lock #

Total comments: 4

Patch Set 17 : #

Patch Set 18 : Rebase and without cc changes #

Total comments: 2

Patch Set 19 : Just removing code #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -417 lines) Patch
M content/renderer/media/stream_texture_factory_impl_android.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 5 chunks +0 lines, -23 lines 0 comments Download
M media/base/video_frame.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +0 lines, -1 line 0 comments Download
M webkit/compositor_bindings/compositor_bindings.gyp View 1 2 3 4 5 6 7 8 2 chunks +0 lines, -4 lines 0 comments Download
M webkit/compositor_bindings/web_compositor_support_impl.h View 1 1 chunk +0 lines, -2 lines 0 comments Download
M webkit/compositor_bindings/web_compositor_support_impl.cc View 1 3 chunks +0 lines, -8 lines 0 comments Download
D webkit/compositor_bindings/web_to_ccvideo_frame_provider.h View 1 1 chunk +0 lines, -44 lines 0 comments Download
D webkit/compositor_bindings/web_to_ccvideo_frame_provider.cc View 1 1 chunk +0 lines, -78 lines 0 comments Download
D webkit/compositor_bindings/web_video_layer_impl.h View 1 1 chunk +0 lines, -38 lines 0 comments Download
D webkit/compositor_bindings/web_video_layer_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -29 lines 0 comments Download
M webkit/media/android/stream_texture_factory_android.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +0 lines, -9 lines 0 comments Download
M webkit/media/android/webmediaplayer_android.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 4 chunks +0 lines, -18 lines 0 comments Download
M webkit/media/android/webmediaplayer_android.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 6 chunks +0 lines, -36 lines 0 comments Download
M webkit/media/webkit_media.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +0 lines, -2 lines 0 comments Download
M webkit/media/webmediaplayer_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +0 lines, -7 lines 0 comments Download
M webkit/media/webmediaplayer_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 6 chunks +0 lines, -31 lines 0 comments Download
M webkit/media/webmediaplayer_ms.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 3 chunks +0 lines, -8 lines 0 comments Download
M webkit/media/webmediaplayer_ms.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 6 chunks +0 lines, -31 lines 0 comments Download
D webkit/media/webvideoframe_impl.h View 1 chunk +0 lines, -31 lines 0 comments Download
D webkit/media/webvideoframe_impl.cc View 1 chunk +0 lines, -17 lines 0 comments Download

Messages

Total messages: 27 (0 generated)
danakj
7 years, 9 months ago (2013-03-16 00:29:14 UTC) #1
danakj
I don't understand WebMediaPlayerImpl vs WebMediaPlayerMS, so I just duplicated the changes I made in ...
7 years, 9 months ago (2013-03-16 00:30:18 UTC) #2
scherkus (not reviewing)
WebMediaPlayerMS is WebRTC's implementation MS = MediaStream ... yeah it should probably be renamed :\
7 years, 9 months ago (2013-03-18 17:17:14 UTC) #3
scherkus (not reviewing)
I'm not the most familiar w/ this code path anymore, but it's looking good https://codereview.chromium.org/12902002/diff/5001/webkit/compositor_bindings/web_layer_impl.cc ...
7 years, 9 months ago (2013-03-18 21:38:24 UTC) #4
danakj
https://codereview.chromium.org/12902002/diff/5001/webkit/compositor_bindings/web_layer_impl.cc File webkit/compositor_bindings/web_layer_impl.cc (right): https://codereview.chromium.org/12902002/diff/5001/webkit/compositor_bindings/web_layer_impl.cc#newcode329 webkit/compositor_bindings/web_layer_impl.cc:329: bool WebLayerImpl::isOrphan() const { return !layer_->layer_tree_host(); } On 2013/03/18 ...
7 years, 9 months ago (2013-03-18 21:57:59 UTC) #5
scherkus (not reviewing)
https://codereview.chromium.org/12902002/diff/5001/webkit/compositor_bindings/web_layer_impl.cc File webkit/compositor_bindings/web_layer_impl.cc (right): https://codereview.chromium.org/12902002/diff/5001/webkit/compositor_bindings/web_layer_impl.cc#newcode329 webkit/compositor_bindings/web_layer_impl.cc:329: bool WebLayerImpl::isOrphan() const { return !layer_->layer_tree_host(); } On 2013/03/18 ...
7 years, 9 months ago (2013-03-18 23:34:18 UTC) #6
danakj
https://codereview.chromium.org/12902002/diff/5001/webkit/media/webmediaplayer_impl.cc File webkit/media/webmediaplayer_impl.cc (right): https://codereview.chromium.org/12902002/diff/5001/webkit/media/webmediaplayer_impl.cc#newcode220 webkit/media/webmediaplayer_impl.cc:220: setStreamTextureClient(NULL); On 2013/03/18 23:34:18, scherkus wrote: > On 2013/03/18 ...
7 years, 9 months ago (2013-03-18 23:37:15 UTC) #7
scherkus (not reviewing)
> > (note: I just remembered you'll have to fix up the Android WebMediaPlayers as ...
7 years, 9 months ago (2013-03-19 00:02:01 UTC) #8
danakj
+qinmin OK that simplifies things lots! This includes changes to the WMPAndroid now. Also removes ...
7 years, 9 months ago (2013-03-19 00:23:34 UTC) #9
qinmin
7 years, 9 months ago (2013-03-19 02:51:54 UTC) #10
jamesr
I'm a little worried about this patch moving the destruction of the media::VideoFrame for WebMediaPlayerMS ...
7 years, 9 months ago (2013-03-21 00:59:11 UTC) #11
danakj
On 2013/03/21 00:59:11, jamesr wrote: > I'm a little worried about this patch moving the ...
7 years, 9 months ago (2013-03-21 03:05:16 UTC) #12
danakj
https://codereview.chromium.org/12902002/diff/41024/webkit/media/android/webmediaplayer_android.h File webkit/media/android/webmediaplayer_android.h (right): https://codereview.chromium.org/12902002/diff/41024/webkit/media/android/webmediaplayer_android.h#newcode248 webkit/media/android/webmediaplayer_android.h:248: cc::VideoFrameProvider::Client* video_frame_provider_client_; On 2013/03/21 00:59:11, jamesr wrote: > Most ...
7 years, 9 months ago (2013-03-21 03:05:26 UTC) #13
danakj
qinmin@ could you please review the android specific parts of this CL now that it's ...
7 years, 9 months ago (2013-03-21 04:23:54 UTC) #14
scherkus (not reviewing)
lgtm++ w/ nits qinmin should still take a peek at the android / stream texture ...
7 years, 9 months ago (2013-03-21 20:38:59 UTC) #15
qinmin
lgtm for android related changed On 2013/03/21 20:38:59, scherkus wrote: > lgtm++ w/ nits > ...
7 years, 9 months ago (2013-03-21 20:50:36 UTC) #16
no sievers
lgtm
7 years, 9 months ago (2013-03-21 21:43:13 UTC) #17
danakj
Plan is: cc/ changes are going here: https://codereview.chromium.org/12596015 removing some headers is going here: https://codereview.chromium.org/12895006/ ...
7 years, 9 months ago (2013-03-21 21:50:47 UTC) #18
scherkus (not reviewing)
On 2013/03/21 03:05:16, danakj wrote: > On 2013/03/21 00:59:11, jamesr wrote: > > I'm a ...
7 years, 9 months ago (2013-03-21 21:55:49 UTC) #19
wjia(left Chromium)
On Thu, Mar 21, 2013 at 2:55 PM, <scherkus@chromium.org> wrote: > On 2013/03/21 03:05:16, danakj ...
7 years, 9 months ago (2013-03-21 23:21:26 UTC) #20
danakj
On Thu, Mar 21, 2013 at 7:21 PM, Wei Jia <wjia@chromium.org> wrote: > > > ...
7 years, 9 months ago (2013-03-21 23:24:13 UTC) #21
danakj
This CL just deletes code now. James PTAL.
7 years, 9 months ago (2013-03-25 23:28:21 UTC) #22
danakj
oh, sherkus as well for media/ please.
7 years, 9 months ago (2013-03-25 23:28:50 UTC) #23
jamesr
lgtm!!!
7 years, 9 months ago (2013-03-25 23:31:01 UTC) #24
scherkus (not reviewing)
lgtm x1000
7 years, 9 months ago (2013-03-26 02:52:01 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/12902002/61001
7 years, 9 months ago (2013-03-26 15:00:14 UTC) #26
commit-bot: I haz the power
7 years, 9 months ago (2013-03-26 17:16:04 UTC) #27
Message was sent while issue was closed.
Change committed as 190706

Powered by Google App Engine
This is Rietveld 408576698