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

Issue 1031583002: Revert of Linux Video Capture: Add V4L2VideoCaptureDelegate{Single,Multi}Plane. (Closed)

Created:
5 years, 9 months ago by perkj_chrome
Modified:
5 years, 9 months ago
CC:
chromium-reviews, posciak+watch_chromium.org, jam, mcasas+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, wjia+watch_chromium.org, miu+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Linux Video Capture: Add V4L2VideoCaptureDelegate{Single,Multi}Plane. (patchset #11 id:630001 of https://codereview.chromium.org/967793002/) Reason for revert: Seems to breaks bots that use real cameras: https://build.chromium.org/p/chromium.webrtc/builders/Linux%20Tester/builds/136 WebRtcPerfBrowserTest.MANUAL_RunsAudioVideoCall60SecsAndLogsInternalMetrics WebRtcPerfBrowserTest.MANUAL_RunsOneWayCall60SecsAndLogsInternalMetrics Original issue's description: > Linux Video Capture: Add V4L2VideoCaptureDelegate{Single,Multi}Plane. > > This CL adds support for V4L2 MPLANE Capture Api. > Only supported format is YUV420M triplanar. > > A new method is added to VideoCaptureDeviceClient, > namely OnIncomingCapturedYuvData(...), which forces > adding MOCKing here and there, and its own > implementation. > > V4L2 MMAP API works via user mmap()ing a number of > buffer allocated by V4L2 capture device. If those > buffers are not correctly munmap()ed, bad things (c) > happen. In light of this, the manual buffer lifetime > management is changed to automatic one. Construction > (mmap()ing) of those called BufferTracker is > planarity specific (i.e. there's one such ctor in > each of BufferTracker{S,M}Plane), while the dtor > is generic and the same. > > ToT class diagram: > +------------------------------------+ > | VideoCaptureDeviceLinux | > | +----------------------+| > | <<ref>> -->| V4L2CaptureDelegate || > | cnt | (struct Buffer) || > | +----------------------+| > +------------------------------------+ > > This CL class scheme: > > +--------------------------+ > | VideoCaptureDeviceLinux | > | | > | <<ref_cnt>> ---+ | > +----------------|---------+ > +----------------v-----------+ v4l2_capture_delegate.{cc,h} > | +-----------------------+ | > | |V4L2CaptureDelegate | | > | | (class BufferTracker)| | > | +-----------------------+ | > +-------^------------------^-+ > | | > +----|-------+ +--------|--+ v4l2_capture_delegate_multi_plane.{cc,h} > | SPlane | | MPlane | > | (BTSplane) | | (BTMPlane)| > | | +-----------+ > +------------+ v4l2_capture_delegate_single_plane.{cc,h} > > - VCDevice works on the premise that its calls into > VCDevice::Client::OnIncomingWhatever() are synchronous. > That assumption is respected here. > > - A bit of cleanup is done in OnIncomingCaptureData(), > in what regards rotation/crop/odd sizes. A unit test > is subsequently added. > > - VideoCaptureDeviceFactory labels the devices as > Single or Multi Planar. That labeling capture_api_type() > needs to ripple through a bunch of files, causing some > otherwise uninteresting changes in the patchsets. > > BUG=441836 > > TEST= Compile and insmod vivid.ko into a kernel, > with options for supporting MPLANE api (multiplanar=2) > then capture using patched Chromium. Current vivid > does _not_ support any common Mplane format, needs > a patch: > > https://github.com/miguelao/linux/tree/adding_yu12_yv12_nv12_nv21___mplane_formats___with_mods_for_kernel_3_13 > > that needs to be compiled against ubuntu sources 3.13 etc. > > For even better coverage, use a normal WebCam and > navigate to http://goo.gl/fUcIiP, then open both > the MPlane camera mentioned in the previous paragraph > and the "normal" webcam (this is,partially, how I > try it). > > Committed: https://crrev.com/d3d37d0f3493dd28cd687fe9285b0aac81a61dae > Cr-Commit-Position: refs/heads/master@{#321612} TBR=emircan@chromium.org,posciak@chromium.org,magjed@chromium.org,dalecurtis@chromium.org,mcasas@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=441836 Committed: https://crrev.com/ec793ac608a5c09baae573d9a488d466181f6531 Cr-Commit-Position: refs/heads/master@{#321746}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+505 lines, -1233 lines) Patch
M content/browser/media/capture/desktop_capture_device_aura_unittest.cc View 1 chunk +0 lines, -10 lines 0 comments Download
M content/browser/media/capture/desktop_capture_device_unittest.cc View 1 chunk +0 lines, -10 lines 0 comments Download
M content/browser/media/capture/web_contents_video_capture_device_unittest.cc View 1 chunk +0 lines, -12 lines 0 comments Download
M content/browser/media/media_internals.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/media/media_internals_unittest.cc View 4 chunks +7 lines, -14 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_controller_unittest.cc View 9 chunks +26 lines, -89 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_device_client.h View 1 chunk +0 lines, -9 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_device_client.cc View 4 chunks +23 lines, -87 lines 0 comments Download
M media/BUILD.gn View 2 chunks +0 lines, -13 lines 0 comments Download
M media/media.gyp View 2 chunks +0 lines, -11 lines 0 comments Download
M media/video/capture/fake_video_capture_device_factory.cc View 1 chunk +1 line, -3 lines 0 comments Download
M media/video/capture/fake_video_capture_device_unittest.cc View 2 chunks +0 lines, -12 lines 0 comments Download
D media/video/capture/linux/v4l2_capture_delegate.h View 1 chunk +0 lines, -150 lines 0 comments Download
D media/video/capture/linux/v4l2_capture_delegate.cc View 1 chunk +0 lines, -419 lines 0 comments Download
D media/video/capture/linux/v4l2_capture_delegate_multi_plane.h View 1 chunk +0 lines, -59 lines 0 comments Download
D media/video/capture/linux/v4l2_capture_delegate_multi_plane.cc View 1 chunk +0 lines, -99 lines 0 comments Download
D media/video/capture/linux/v4l2_capture_delegate_single_plane.h View 1 chunk +0 lines, -54 lines 0 comments Download
D media/video/capture/linux/v4l2_capture_delegate_single_plane.cc View 1 chunk +0 lines, -60 lines 0 comments Download
M media/video/capture/linux/video_capture_device_factory_linux.cc View 4 chunks +8 lines, -15 lines 0 comments Download
M media/video/capture/linux/video_capture_device_linux.h View 2 chunks +2 lines, -5 lines 0 comments Download
M media/video/capture/linux/video_capture_device_linux.cc View 4 chunks +424 lines, -39 lines 0 comments Download
M media/video/capture/video_capture_device.h View 6 chunks +7 lines, -26 lines 0 comments Download
M media/video/capture/video_capture_device.cc View 3 chunks +5 lines, -22 lines 0 comments Download
M media/video/capture/video_capture_device_unittest.cc View 3 chunks +1 line, -14 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
perkj_chrome
Created Revert of Linux Video Capture: Add V4L2VideoCaptureDelegate{Single,Multi}Plane.
5 years, 9 months ago (2015-03-23 08:53:03 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1031583002/1
5 years, 9 months ago (2015-03-23 08:53:23 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 9 months ago (2015-03-23 08:54:14 UTC) #3
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/ec793ac608a5c09baae573d9a488d466181f6531 Cr-Commit-Position: refs/heads/master@{#321746}
5 years, 9 months ago (2015-03-23 08:55:27 UTC) #4
perkj_chrome
5 years, 9 months ago (2015-03-23 08:59:43 UTC) #5
Message was sent while issue was closed.
On 2015/03/23 08:54:14, I haz the power (commit-bot) wrote:
> Committed patchset #1 (id:1)

Sorry, the broken tests use file video capture of course, not a real camera.
switches::kUseFileForFakeVideoCapture.

Powered by Google App Engine
This is Rietveld 408576698