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

Issue 2772963002: Reland [Mojo Video Capture] Add test coverage for accelerated jpeg decoding (Closed)

Created:
3 years, 9 months ago by chfremer
Modified:
3 years, 8 months ago
Reviewers:
emircan, mcasas
CC:
chromium-reviews, qsr+mojo_chromium.org, Aaron Boodman, posciak+watch_chromium.org, chfremer+watch_chromium.org, viettrungluu+watch_chromium.org, jam, yzshen+watch_chromium.org, abarth-chromium, feature-media-reviews_chromium.org, darin-cc_chromium.org, piman+watch_chromium.org, xjz+watch_chromium.org, darin (slow to review), miu+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Reland [Mojo Video Capture] Add test coverage for accelerated jpeg decoding PatchSet 1 is state as reviewed previously in https://codereview.chromium.org/2735083002/ PatchSet 2 fixes the reason for the revert. Description of issue and fix: VideoCaptureBrowserTest.ReceiveFramesFromFakeCaptureDevice never called VideoCaptureController::ReturnBuffer(). As a result, after the maximum number of allowed in-flight buffers (which is 3) had been reached, subsequent frames were dropped. The tests using the software decoding path would always pass, because kMinFramesToReceive was set to 3. It would have failed if set to >3. The new test case for the accelerated decoding path would pass if the fake decoder would be initialized and used within the first 3 frames. It would fail otherwise. The fix is to call VideoCaptureController::ReturnBuffer() when receiving a frame. To allow calling this synchronously from within an invokaction of OnBufferReady(), the order of corresponding invocations in VideoCaptureController had to be updated. Description of original CL: This CL adds test cases to VideoCaptureBrowserTest that exercise the code path for accelerated jpeg decoding using a fake decode accelerator. * Add a FakeJpegDecodeAccelerator that can run in the GPU process and decodes jpeg frames in software using libyuv. * Add a command-line flag kUseFakeJpegDecodeAccelerator to enable the fake accelerator (and suppress any "real" ones if present). * Fix threading-related DCHECKs in GpuJpegDecodeAccelerator::Client to match what is actually happening. This is needed to make the new test cases pass. It also fixes issue 688184 [1]. This CL is part of the Mojo Video Capture work. For the bigger picture, see [2] CL14. BUG=584797, 688184 TEST= content_browsertests --gtest_filter="VideoCaptureBrowserTest.*" CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel TBR=posciak@chromium.org,piman@chromium.org,sandersd@chromium.org Review-Url: https://codereview.chromium.org/2772963002 Cr-Commit-Position: refs/heads/master@{#461060} Committed: https://chromium.googlesource.com/chromium/src/+/0f72e9da2531d02d80511e54584b6da18ffa41d3

Patch Set 1 : State as previously reviewed #

Patch Set 2 : Fix bug in VideoCaptureBrowserTest #

Patch Set 3 : Rebase to March 28 #

Patch Set 4 : Merge-in CL for fixing DCHECK being hit on trybot and rebase to March 29 #

Patch Set 5 : Rebase to March 30 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+353 lines, -52 lines) Patch
M content/browser/gpu/gpu_process_host.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_browsertest.cc View 1 7 chunks +45 lines, -11 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_controller.h View 2 chunks +6 lines, -0 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_controller.cc View 1 5 chunks +32 lines, -14 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_controller_event_handler.h View 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_controller_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.cc View 2 chunks +4 lines, -1 line 0 comments Download
M content/browser/renderer_host/media/video_capture_host.h View 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_host.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_manager_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/media/video_frame_receiver_on_io_thread.h View 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/media/video_frame_receiver_on_io_thread.cc View 1 chunk +6 lines, -0 lines 0 comments Download
M media/base/media_switches.h View 1 chunk +1 line, -0 lines 0 comments Download
M media/base/media_switches.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M media/capture/video/video_capture_device_client.h View 1 chunk +1 line, -0 lines 0 comments Download
M media/capture/video/video_capture_device_client.cc View 3 chunks +9 lines, -4 lines 0 comments Download
M media/capture/video/video_capture_device_client_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M media/capture/video/video_frame_receiver.h View 1 chunk +1 line, -0 lines 0 comments Download
M media/gpu/BUILD.gn View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A media/gpu/fake_jpeg_decode_accelerator.h View 1 chunk +69 lines, -0 lines 0 comments Download
A media/gpu/fake_jpeg_decode_accelerator.cc View 1 chunk +103 lines, -0 lines 0 comments Download
M media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.cc View 1 2 3 4 4 chunks +18 lines, -9 lines 0 comments Download
M media/gpu/ipc/service/gpu_jpeg_decode_accelerator.cc View 7 chunks +36 lines, -13 lines 0 comments Download
M services/video_capture/receiver_mojo_to_media_adapter.h View 1 chunk +1 line, -0 lines 0 comments Download
M services/video_capture/receiver_mojo_to_media_adapter.cc View 1 chunk +4 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 28 (21 generated)
chfremer
mcasas@: PTAL emircan@: FYI
3 years, 9 months ago (2017-03-23 22:57:11 UTC) #3
mcasas
ps1 deltas lgtm
3 years, 9 months ago (2017-03-24 20:33:20 UTC) #5
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/2772963002/20001
3 years, 9 months ago (2017-03-24 20:35:24 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/257551)
3 years, 9 months ago (2017-03-24 23:15:08 UTC) #9
chfremer
On 2017/03/24 23:15:08, commit-bot: I haz the power wrote: > Try jobs failed on following ...
3 years, 8 months ago (2017-03-28 23:08:30 UTC) #10
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/2772963002/80001
3 years, 8 months ago (2017-03-31 06:45:03 UTC) #25
commit-bot: I haz the power
3 years, 8 months ago (2017-03-31 06:51:34 UTC) #28
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/0f72e9da2531d02d80511e54584b...

Powered by Google App Engine
This is Rietveld 408576698