Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(7)

Issue 2700173002: Add MJPEG support to FakeVideoCaptureDevice (Closed)

Created:
11 months, 1 week ago by chfremer
Modified:
11 months ago
CC:
chromium-reviews, posciak+watch_chromium.org, chfremer+watch_chromium.org, feature-media-reviews_chromium.org, mcasas+watch+vc_chromium.org, xjz+watch_chromium.org, miu+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add MJPEG support to FakeVideoCaptureDevice Part of a series of CLs for FakeVideoCaptureDevice with the goal of allowing us to expand our test coverage of the video capture pipeline. This CL is part of the Mojo Video Capture work. For the bigger picture, see [1] CL12. BUG=584797 TEST= capture_unittests --gtest_filter="*Video*" [1] https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxsxWslJ_X-zpOhijvI8/edit?usp=sharing Review-Url: https://codereview.chromium.org/2700173002 Cr-Commit-Position: refs/heads/master@{#452674} Committed: https://chromium.googlesource.com/chromium/src/+/98985be5a81f6862fa536c05cda2292a36f18e8b

Patch Set 1 #

Total comments: 25

Patch Set 2 : mcasas@ suggestions #

Patch Set 3 : rebase and format #

Patch Set 4 : Fix for compilers on Win and Android #

Total comments: 14

Patch Set 5 : mcasas@ suggestions #

Total comments: 1

Patch Set 6 : Add a const #

Unified diffs Side-by-side diffs Delta from patch set Stats (+252 lines, -137 lines) Patch
M media/capture/video/fake_video_capture_device.h View 1 2 3 4 2 chunks +6 lines, -1 line 0 comments Download
M media/capture/video/fake_video_capture_device.cc View 1 2 3 4 18 chunks +183 lines, -104 lines 0 comments Download
M media/capture/video/fake_video_capture_device_factory.cc View 1 2 3 4 4 chunks +26 lines, -13 lines 0 comments Download
M media/capture/video/fake_video_capture_device_unittest.cc View 1 2 3 4 5 8 chunks +37 lines, -19 lines 0 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 37 (27 generated)
chfremer
mcasas@: PTAL emircan@: FYI guidou@: FYI
11 months, 1 week ago (2017-02-17 17:48:17 UTC) #6
chfremer
https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device.cc File media/capture/video/fake_video_capture_device.cc (right): https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device.cc#newcode175 media/capture/video/fake_video_capture_device.cc:175: base::TimeTicks first_ref_time_; I forgot to remove members |device_state|, |client_| ...
11 months, 1 week ago (2017-02-17 18:07:38 UTC) #7
mcasas
https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device.cc File media/capture/video/fake_video_capture_device.cc (right): https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device.cc#newcode275 media/capture/video/fake_video_capture_device.cc:275: } I find this part hard to read, mostly ...
11 months, 1 week ago (2017-02-17 18:35:47 UTC) #10
chfremer
PTAL Incorporated suggestions. https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device.cc File media/capture/video/fake_video_capture_device.cc (right): https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device.cc#newcode175 media/capture/video/fake_video_capture_device.cc:175: base::TimeTicks first_ref_time_; On 2017/02/17 18:07:38, chfremer ...
11 months ago (2017-02-22 17:28:42 UTC) #13
mcasas
https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device_factory.h File media/capture/video/fake_video_capture_device_factory.h (right): https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device_factory.h#newcode27 media/capture/video/fake_video_capture_device_factory.h:27: static media::VideoPixelFormat GetPixelFormatFromDeviceId( On 2017/02/22 17:28:42, chfremer wrote: > ...
11 months ago (2017-02-22 20:26:33 UTC) #22
chfremer
PTAL https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device_factory.h File media/capture/video/fake_video_capture_device_factory.h (right): https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device_factory.h#newcode27 media/capture/video/fake_video_capture_device_factory.h:27: static media::VideoPixelFormat GetPixelFormatFromDeviceId( On 2017/02/22 20:26:32, mcasas wrote: ...
11 months ago (2017-02-23 01:14:28 UTC) #26
mcasas
ps5 lgtm https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device_factory.h File media/capture/video/fake_video_capture_device_factory.h (right): https://codereview.chromium.org/2700173002/diff/1/media/capture/video/fake_video_capture_device_factory.h#newcode27 media/capture/video/fake_video_capture_device_factory.h:27: static media::VideoPixelFormat GetPixelFormatFromDeviceId( On 2017/02/23 01:14:27, chfremer ...
11 months ago (2017-02-23 21:46:19 UTC) #30
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/2700173002/100001
11 months ago (2017-02-23 22:16:05 UTC) #33
commit-bot: I haz the power
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/98985be5a81f6862fa536c05cda2292a36f18e8b
11 months ago (2017-02-23 23:40:27 UTC) #36
Alexander Alekseev
11 months ago (2017-02-25 03:33:45 UTC) #37
Message was sent while issue was closed.
A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/2712123003/ by alemate@chromium.org.

The reason for reverting is: Speculative revert because of
video_ChromeRTCHWDecodeUsed failure on PFQ informational builders:

[4316:4316:0224/131423.612615:ERROR:v4l2_jpeg_decode_accelerator.cc(206)] Failed
to open device.

Powered by Google App Engine
This is Rietveld 408576698