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

Issue 2700173002: Add MJPEG support to FakeVideoCaptureDevice (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 months ago by chfremer
Modified:
5 months, 3 weeks 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
6 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_| ...
6 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 ...
6 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 ...
5 months, 3 weeks 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: > ...
5 months, 3 weeks 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: ...
5 months, 3 weeks 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 months, 3 weeks 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 months, 3 weeks 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 months, 3 weeks ago (2017-02-23 23:40:27 UTC) #36
Alexander Alekseev
5 months, 3 weeks 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 b40b6558b