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

Issue 2700173002: Add MJPEG support to FakeVideoCaptureDevice (Closed)

Created:
1 year, 2 months ago by chfremer
Modified:
1 year, 1 month 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
1 year, 2 months 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_| ...
1 year, 2 months 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 ...
1 year, 2 months 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 ...
1 year, 2 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: > ...
1 year, 2 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: ...
1 year, 2 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 ...
1 year, 1 month 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
1 year, 1 month 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
1 year, 1 month ago (2017-02-23 23:40:27 UTC) #36
Alexander Alekseev
1 year, 1 month 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