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

Issue 2428263004: 16 bpp video stream capture, render and createImageBitmap(video) using (CPU) shared memory buffers (Closed)

Created:
4 years, 2 months ago by aleksandar.stojiljkovic
Modified:
4 years, 1 month ago
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, posciak+watch_chromium.org, blink-reviews-html_chromium.org, jam, mlamouri+watch-blink_chromium.org, eric.carlson_apple.com, blink-reviews-api_chromium.org, haraken, feature-media-reviews_chromium.org, dglazkov+blink, darin-cc_chromium.org, mcasas+watch+vc_chromium.org, blink-reviews, cc-bugs_chromium.org, Srirama, miu+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

16 bpp video stream capture, render and createImageBitmap(video) using (CPU) shared memory buffers This patch implements support for depth capture on Windows/Linux and ChromeOS using shared memory buffers. WebGL implementation with no precision loss and GPU memory buffer usage is split to separate patches from master patch: crrev.com/2121043002/ It Supports 16 bit depth video streams from R200 and SR300. Verified to work on Windows 10 and Ubuntu 16.04. Rendering video element: In cc, it is converted to RGBA format before glTexImage2D and rendered as TexturedQuad. 2D canvas + getImageData returns 8 bit color components (higher bit value) in RGBA. BUG=624436 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Committed: https://crrev.com/198b050b9fe647bf3799ab75df07b888c7adcf50 Cr-Commit-Position: refs/heads/master@{#429307}

Patch Set 1 #

Patch Set 2 : fixes #

Total comments: 52

Patch Set 3 : Review #19 fixes. Thanks mcasas@. #

Patch Set 4 : rebase #

Patch Set 5 : fix windows approach and video_capture_utils. #

Total comments: 12

Patch Set 6 : review #27 fixes. Thanks danakj@. #

Patch Set 7 : review #30 fix. Thanks danakj@. #

Total comments: 15

Patch Set 8 : Review #33 and #34 fixes. Removed WebGL part of code. Thanks danakj@ and mcasas@. #

Total comments: 22

Patch Set 9 : Rebase after FakeVCD (2447233002) land. #

Total comments: 1

Patch Set 10 : review #42 & #43 fixes. Thanks xhwang@ and hubbe@. #

Patch Set 11 : video_frame.cc switch fix. #

Total comments: 8

Patch Set 12 : #38 fix: refactoring VideoCaptureDeviceClient::OnIncomingCapturedY16Data out. Thanks mcasas@. #

Total comments: 1

Patch Set 13 : #48 fix: Y16 SkCanvasVideoRendererTest. Thanks xhwang@. #

Total comments: 6

Patch Set 14 : Fixes. #

Patch Set 15 : Set FakeVCD count-devices=2 for webrtc_getusermedia_browsertest only. #

Total comments: 3

Patch Set 16 : Split webrtc_depth_capture_browsertest. Thanks phoglund@, #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+712 lines, -141 lines) Patch
M cc/output/renderer_pixeltest.cc View 1 2 3 4 5 6 7 11 chunks +152 lines, -27 lines 0 comments Download
M cc/resources/video_resource_updater.cc View 1 2 3 4 5 6 7 8 1 chunk +13 lines, -7 lines 0 comments Download
A cc/test/data/intersecting_light_dark_squares_video.png View Binary file 0 comments Download
M content/browser/renderer_host/media/video_capture_buffer_pool_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_controller.cc View 1 2 3 2 chunks +5 lines, -3 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_controller_unittest.cc View 1 2 3 4 5 6 7 8 14 chunks +57 lines, -57 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_device_client_unittest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/renderer_host/media/video_capture_host.cc View 1 2 3 1 chunk +3 lines, -2 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_manager.cc View 1 2 3 4 5 6 7 1 chunk +4 lines, -3 lines 0 comments Download
A content/browser/webrtc/webrtc_depth_capture_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +77 lines, -0 lines 0 comments Download
M content/renderer/media/video_capture_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +9 lines, -4 lines 0 comments Download
M content/test/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
A content/test/data/media/depth_stream_test_utilities.js View 1 2 1 chunk +111 lines, -0 lines 2 comments Download
A content/test/data/media/getusermedia-depth-capture.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +56 lines, -0 lines 0 comments Download
M media/base/video_frame.cc View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +28 lines, -14 lines 0 comments Download
M media/capture/video/linux/v4l2_capture_delegate.cc View 1 2 3 4 5 4 chunks +21 lines, -3 lines 0 comments Download
M media/capture/video/video_capture_device_client.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +7 lines, -0 lines 0 comments Download
M media/capture/video/video_capture_device_client.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 6 chunks +45 lines, -6 lines 0 comments Download
M media/capture/video/win/sink_filter_win.h View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -0 lines 0 comments Download
M media/capture/video/win/sink_filter_win.cc View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
M media/capture/video/win/sink_input_pin_win.cc View 1 2 3 4 2 chunks +18 lines, -2 lines 0 comments Download
M media/capture/video/win/video_capture_device_factory_win.cc View 1 chunk +3 lines, -1 line 0 comments Download
M media/capture/video/win/video_capture_device_mf_win.cc View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
M media/capture/video/win/video_capture_device_win.cc View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M media/renderers/skcanvas_video_renderer.h View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M media/renderers/skcanvas_video_renderer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +29 lines, -10 lines 0 comments Download
M media/renderers/skcanvas_video_renderer_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +45 lines, -0 lines 0 comments Download

Messages

Total messages: 75 (29 generated)
aleksandar.stojiljkovic
danakj@chromium.org: Please review changes in cc, sky@chromium.org: Please review changes in content/test and WebKit/public, kbr@chromium.org: ...
4 years, 2 months ago (2016-10-20 21:26:34 UTC) #12
aleksandar.stojiljkovic
danakj@, short introduction to cc changes. https://codereview.chromium.org/2428263004/diff/20001/cc/output/renderer_pixeltest.cc File cc/output/renderer_pixeltest.cc (right): https://codereview.chromium.org/2428263004/diff/20001/cc/output/renderer_pixeltest.cc#newcode309 cc/output/renderer_pixeltest.cc:309: TextureDrawQuad* quad = ...
4 years, 2 months ago (2016-10-20 21:52:49 UTC) #13
Avi (use Gerrit)
The content code LGTM. Please also have a media expert review it.
4 years, 2 months ago (2016-10-20 21:59:09 UTC) #14
aleksandar.stojiljkovic
On 2016/10/20 21:59:09, Avi wrote: > The content code LGTM. > > Please also have ...
4 years, 2 months ago (2016-10-20 22:26:27 UTC) #16
sky
content/test/data has an OWNERS of *, so you don't need me. I'm not a good ...
4 years, 2 months ago (2016-10-20 22:58:15 UTC) #18
mcasas
Took a first pass. Some generalities: - Try hard not to have special cases for ...
4 years, 2 months ago (2016-10-21 00:10:51 UTC) #19
aleksandar.stojiljkovic
Patch Set 3 : Review #19 fixes. Thanks mcasas@chromium.org Addressed the issues, except windows GUIDs ...
4 years, 2 months ago (2016-10-21 22:11:11 UTC) #20
aleksandar.stojiljkovic
Patchset 3, 4 and 5 are addressing issues from review #19. Simplified the code further ...
4 years, 1 month ago (2016-10-24 14:57:49 UTC) #21
aleksandar.stojiljkovic
+enne@chromium.org: Please review changes in cc. danakj@ seems busy. Thanks.
4 years, 1 month ago (2016-10-24 19:13:19 UTC) #24
mcasas
I cannot see media/capture/video/win/sink_filter_win.* files: when I click on them it says "old chunk mismatch". ...
4 years, 1 month ago (2016-10-24 19:48:47 UTC) #25
Ken Russell (switch to Gerrit)
The WebGL changes in WebKit/Source look fine. Could you please find another reviewer for content/browser/renderer_host? ...
4 years, 1 month ago (2016-10-24 20:08:17 UTC) #26
danakj
https://codereview.chromium.org/2428263004/diff/20001/cc/output/renderer_pixeltest.cc File cc/output/renderer_pixeltest.cc (right): https://codereview.chromium.org/2428263004/diff/20001/cc/output/renderer_pixeltest.cc#newcode288 cc/output/renderer_pixeltest.cc:288: void CreateTestY16VideoDrawQuad_FromVideoFrame( s/VideoDrawQuad/TextureDrawQuad/ ? https://codereview.chromium.org/2428263004/diff/80001/cc/output/renderer_pixeltest.cc File cc/output/renderer_pixeltest.cc (right): https://codereview.chromium.org/2428263004/diff/80001/cc/output/renderer_pixeltest.cc#newcode855 ...
4 years, 1 month ago (2016-10-24 21:46:37 UTC) #27
aleksandar.stojiljkovic
On 2016/10/24 20:08:17, Ken Russell wrote: > The WebGL changes in WebKit/Source look fine. > ...
4 years, 1 month ago (2016-10-25 10:11:09 UTC) #28
aleksandar.stojiljkovic
Patch Set 6 : review #27 fixes. Thanks danakj@. https://codereview.chromium.org/2428263004/diff/20001/cc/output/renderer_pixeltest.cc File cc/output/renderer_pixeltest.cc (right): https://codereview.chromium.org/2428263004/diff/20001/cc/output/renderer_pixeltest.cc#newcode288 cc/output/renderer_pixeltest.cc:288: ...
4 years, 1 month ago (2016-10-25 10:12:28 UTC) #29
danakj
https://codereview.chromium.org/2428263004/diff/80001/cc/output/renderer_pixeltest.cc File cc/output/renderer_pixeltest.cc (right): https://codereview.chromium.org/2428263004/diff/80001/cc/output/renderer_pixeltest.cc#newcode855 cc/output/renderer_pixeltest.cc:855: void AppendBackgroundAndRunTest(const PixelComparator& comparator, On 2016/10/25 10:12:28, aleksandar.stojiljkovic wrote: ...
4 years, 1 month ago (2016-10-25 20:21:23 UTC) #30
aleksandar.stojiljkovic
On 2016/10/24 19:48:47, mcasas wrote: > Also, what if we review separately the changes to ...
4 years, 1 month ago (2016-10-25 21:14:59 UTC) #31
aleksandar.stojiljkovic
Patch Set 7: review #30 fix. Thanks danakj@. https://codereview.chromium.org/2428263004/diff/80001/cc/output/renderer_pixeltest.cc File cc/output/renderer_pixeltest.cc (right): https://codereview.chromium.org/2428263004/diff/80001/cc/output/renderer_pixeltest.cc#newcode855 cc/output/renderer_pixeltest.cc:855: void ...
4 years, 1 month ago (2016-10-25 21:51:41 UTC) #32
danakj
https://codereview.chromium.org/2428263004/diff/120001/cc/output/renderer_pixeltest.cc File cc/output/renderer_pixeltest.cc (right): https://codereview.chromium.org/2428263004/diff/120001/cc/output/renderer_pixeltest.cc#newcode571 cc/output/renderer_pixeltest.cc:571: void CreateTestY16VideoDrawQuad_TwoColor( This is making TextureDrawQuad too https://codereview.chromium.org/2428263004/diff/120001/cc/output/renderer_pixeltest.cc#newcode855 cc/output/renderer_pixeltest.cc:855: ...
4 years, 1 month ago (2016-10-25 22:34:19 UTC) #33
mcasas
changes under - content/browser/renderer_host/media/* - content/rendere/media - media/capture/video (except fake* since they are reviewed in ...
4 years, 1 month ago (2016-10-25 22:44:43 UTC) #34
aleksandar.stojiljkovic
To avoid growing this patch, removed Webkit (WebGL) related code and part of skcanvas_video_renderer.* as ...
4 years, 1 month ago (2016-10-26 14:02:18 UTC) #36
danakj
cc LGTM https://codereview.chromium.org/2428263004/diff/140001/cc/output/renderer_pixeltest.cc File cc/output/renderer_pixeltest.cc (right): https://codereview.chromium.org/2428263004/diff/140001/cc/output/renderer_pixeltest.cc#newcode1082 cc/output/renderer_pixeltest.cc:1082: SCOPED_TRACE("IntersectingVideoQuads"); These SCOPED_TRACE are not adding anything, ...
4 years, 1 month ago (2016-10-26 19:59:48 UTC) #37
mcasas
my parts lgtm with comment addressed and pre-landing the Fake* changes CL separately (bc easier ...
4 years, 1 month ago (2016-10-27 00:44:42 UTC) #38
aleksandar.stojiljkovic
+xhwang@chromium.org, Please review changes in media/base/video_frame.cc media/renderers/skcanvas_video_renderer.* Thanks.
4 years, 1 month ago (2016-10-27 08:05:12 UTC) #40
dshwang
non-owner lgtm it's good idea to support Y16 using existing RGBA first. looking forward to ...
4 years, 1 month ago (2016-10-27 08:54:43 UTC) #41
xhwang
I just have a few comments. hubbe: Please also help take a look at the ...
4 years, 1 month ago (2016-10-27 16:59:18 UTC) #42
hubbe
https://codereview.chromium.org/2428263004/diff/140001/media/renderers/skcanvas_video_renderer.cc File media/renderers/skcanvas_video_renderer.cc (right): https://codereview.chromium.org/2428263004/diff/140001/media/renderers/skcanvas_video_renderer.cc#newcode542 media/renderers/skcanvas_video_renderer.cc:542: *rgba++ = SkColorSetARGB(0xFF, green, green, green); I wonder if ...
4 years, 1 month ago (2016-10-27 17:25:03 UTC) #43
aleksandar.stojiljkovic
Patch Set 10 : review #42 & #43 fixes. Thanks xhwang@ and hubbe@. https://codereview.chromium.org/2428263004/diff/140001/media/base/video_frame.cc File ...
4 years, 1 month ago (2016-10-27 19:12:23 UTC) #44
xhwang
Thanks! It looks good to me % a request for a test. https://codereview.chromium.org/2428263004/diff/200001/media/base/video_frame.cc File media/base/video_frame.cc ...
4 years, 1 month ago (2016-10-27 20:28:10 UTC) #45
aleksandar.stojiljkovic
Thanks xhwang@, https://codereview.chromium.org/2428263004/diff/200001/media/base/video_frame.cc File media/base/video_frame.cc (right): https://codereview.chromium.org/2428263004/diff/200001/media/base/video_frame.cc#newcode812 media/base/video_frame.cc:812: return frame; On 2016/10/27 20:28:09, xhwang wrote: ...
4 years, 1 month ago (2016-10-27 20:59:42 UTC) #46
aleksandar.stojiljkovic
On 2016/10/27 20:59:42, aleksandar.stojiljkovic wrote: > Thanks xhwang@, > > https://codereview.chromium.org/2428263004/diff/200001/media/base/video_frame.cc > File media/base/video_frame.cc (right): ...
4 years, 1 month ago (2016-10-27 21:01:13 UTC) #47
xhwang
https://codereview.chromium.org/2428263004/diff/200001/media/base/video_frame.cc File media/base/video_frame.cc (right): https://codereview.chromium.org/2428263004/diff/200001/media/base/video_frame.cc#newcode812 media/base/video_frame.cc:812: return frame; On 2016/10/27 20:59:41, aleksandar.stojiljkovic wrote: > On ...
4 years, 1 month ago (2016-10-27 21:09:06 UTC) #48
aleksandar.stojiljkovic
Patch Set 12 : #38 fix: refactoring VideoCaptureDeviceClient::OnIncomingCapturedY16Data out. Thanks mcasas@. https://codereview.chromium.org/2428263004/diff/140001/cc/output/renderer_pixeltest.cc File cc/output/renderer_pixeltest.cc (right): ...
4 years, 1 month ago (2016-10-27 21:27:22 UTC) #49
aleksandar.stojiljkovic
Patch Set 13 : #48 fix: Y16 SkCanvasVideoRendererTest. Thanks xhwang@. https://codereview.chromium.org/2428263004/diff/200001/media/base/video_frame.cc File media/base/video_frame.cc (right): https://codereview.chromium.org/2428263004/diff/200001/media/base/video_frame.cc#newcode812 ...
4 years, 1 month ago (2016-10-28 13:30:24 UTC) #51
xhwang
Glad the test found real bugs. I usually don't trust my eyes and only trust ...
4 years, 1 month ago (2016-10-28 16:54:17 UTC) #52
aleksandar.stojiljkovic
Thanks. https://codereview.chromium.org/2428263004/diff/120001/content/renderer/media/video_capture_impl.cc File content/renderer/media/video_capture_impl.cc (right): https://codereview.chromium.org/2428263004/diff/120001/content/renderer/media/video_capture_impl.cc#newcode263 content/renderer/media/video_capture_impl.cc:263: DCHECK_EQ(media::PIXEL_STORAGE_CPU, info->storage_type); On 2016/10/26 14:02:18, aleksandar.stojiljkovic wrote: > ...
4 years, 1 month ago (2016-10-28 18:58:55 UTC) #53
aleksandar.stojiljkovic
Patch Set 15 : Set FakeVCD count-devices=2 for webrtc_getusermedia_browsertest only. https://codereview.chromium.org/2428263004/diff/300001/content/browser/webrtc/webrtc_getusermedia_browsertest.cc File content/browser/webrtc/webrtc_getusermedia_browsertest.cc (right): https://codereview.chromium.org/2428263004/diff/300001/content/browser/webrtc/webrtc_getusermedia_browsertest.cc#newcode132 ...
4 years, 1 month ago (2016-10-30 11:08:39 UTC) #56
aleksandar.stojiljkovic
avi@chromium.org, I'll wait for your review on #56. The rest of the changes are reviewed ...
4 years, 1 month ago (2016-10-30 20:35:32 UTC) #59
aleksandar.stojiljkovic
tommi@, in case avi@ is busy and because it is in your domain, too, PTAL ...
4 years, 1 month ago (2016-10-31 18:50:06 UTC) #61
aleksandar.stojiljkovic
phoglund@, PTAL at [1]. The code mentioned in #56 [1] is a change after Avi's ...
4 years, 1 month ago (2016-11-01 18:43:15 UTC) #63
phoglund_chromium
https://codereview.chromium.org/2428263004/diff/300001/content/browser/webrtc/webrtc_getusermedia_browsertest.cc File content/browser/webrtc/webrtc_getusermedia_browsertest.cc (right): https://codereview.chromium.org/2428263004/diff/300001/content/browser/webrtc/webrtc_getusermedia_browsertest.cc#newcode137 content/browser/webrtc/webrtc_getusermedia_browsertest.cc:137: switches::kUseFakeDeviceForMediaStream; Well, your new test does appear to use ...
4 years, 1 month ago (2016-11-02 10:56:57 UTC) #64
aleksandar.stojiljkovic
Patch Set 16 : Split webrtc_depth_capture_browsertest. Thanks phoglund@. https://codereview.chromium.org/2428263004/diff/300001/content/browser/webrtc/webrtc_getusermedia_browsertest.cc File content/browser/webrtc/webrtc_getusermedia_browsertest.cc (right): https://codereview.chromium.org/2428263004/diff/300001/content/browser/webrtc/webrtc_getusermedia_browsertest.cc#newcode137 content/browser/webrtc/webrtc_getusermedia_browsertest.cc:137: switches::kUseFakeDeviceForMediaStream; ...
4 years, 1 month ago (2016-11-02 13:46:44 UTC) #65
phoglund_chromium
lgtm w/ one suggestion https://codereview.chromium.org/2428263004/diff/320001/content/test/data/media/depth_stream_test_utilities.js File content/test/data/media/depth_stream_test_utilities.js (right): https://codereview.chromium.org/2428263004/diff/320001/content/test/data/media/depth_stream_test_utilities.js#newcode35 content/test/data/media/depth_stream_test_utilities.js:35: function testVideoToImageBitmap(videoElementName, success, error) I'd ...
4 years, 1 month ago (2016-11-02 14:52:06 UTC) #66
aleksandar.stojiljkovic
https://codereview.chromium.org/2428263004/diff/320001/content/test/data/media/depth_stream_test_utilities.js File content/test/data/media/depth_stream_test_utilities.js (right): https://codereview.chromium.org/2428263004/diff/320001/content/test/data/media/depth_stream_test_utilities.js#newcode35 content/test/data/media/depth_stream_test_utilities.js:35: function testVideoToImageBitmap(videoElementName, success, error) On 2016/11/02 14:52:05, phoglund_chrome wrote: ...
4 years, 1 month ago (2016-11-02 15:10:42 UTC) #68
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/2428263004/320001
4 years, 1 month ago (2016-11-02 15:11:14 UTC) #71
commit-bot: I haz the power
Committed patchset #16 (id:320001)
4 years, 1 month ago (2016-11-02 16:52:50 UTC) #73
commit-bot: I haz the power
4 years, 1 month ago (2016-11-02 17:21:29 UTC) #75
Message was sent while issue was closed.
Patchset 16 (id:??) landed as
https://crrev.com/198b050b9fe647bf3799ab75df07b888c7adcf50
Cr-Commit-Position: refs/heads/master@{#429307}

Powered by Google App Engine
This is Rietveld 408576698