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

Issue 2700173002: Add MJPEG support to FakeVideoCaptureDevice (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 5 days ago by chfremer
Modified:
5 days, 18 hours 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
Commit queue not available (can’t edit this change).

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 37 (27 generated)
chfremer
mcasas@: PTAL emircan@: FYI guidou@: FYI
1 week, 4 days 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 week, 4 days 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 week, 4 days 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 week 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: > ...
6 days, 21 hours 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: ...
6 days, 16 hours 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 ...
5 days, 20 hours 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
5 days, 19 hours 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
5 days, 18 hours ago (2017-02-23 23:40:27 UTC) #36
Alexander Alekseev
4 days, 14 hours 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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f8e48bd