Cleanup method VideoCaptureManager::GetDeviceFormatsInUse() |
||
Issue description
The method VideoCaptureManager::GetDeviceFormatsInUse() [1] has several issues:
1.) The method signature is wonky: It has a
meaningless bool return value (because it unconditionally returns true), and it modifies a list of things when it could just return the one thing it provides (when available). A better alternative might be:
base::Optional<media::VideoCaptureFormat> GetDeviceFormat(stream_type,
device_id) {
...
return device_in_use ? device_in_use->GetVideoCaptureFormat() :
base::nullopt;
}
2.) The method implementation does not match the specification in its method-level comment.
[1] https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/video_capture_manager.cc?q=VideoCaptureManager&dr=CSs&l=903
,
Apr 5 2017
Yes, please go ahead.
,
Apr 10 2017
Submitted https://codereview.chromium.org/2810723002/. Please review.
,
Apr 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cf466af46f6bc3bd2113e014909f97d1955f12c3 commit cf466af46f6bc3bd2113e014909f97d1955f12c3 Author: c.padhi <c.padhi@samsung.com> Date: Tue Apr 11 17:02:42 2017 Refactor VideoCaptureManager::GetDeviceFormatsInUse() VideoCaptureManager::GetDeviceFormatsInUse() [1] has below issues: 1. It unconditionally returns true and modifies a list of things when it could just return the one thing it provides (when available). 2. Its implementation does not match the specification in its method- level comment. [1] https://cs.chromium.org/chromium/src/content/browser/renderer_host/ media/video_capture_manager.h?type=cs&l=156 This CL takes care of these issues. BUG= 702271 Review-Url: https://codereview.chromium.org/2810723002 Cr-Commit-Position: refs/heads/master@{#463659} [modify] https://crrev.com/cf466af46f6bc3bd2113e014909f97d1955f12c3/content/browser/renderer_host/media/media_devices_dispatcher_host.cc [modify] https://crrev.com/cf466af46f6bc3bd2113e014909f97d1955f12c3/content/browser/renderer_host/media/video_capture_controller.cc [modify] https://crrev.com/cf466af46f6bc3bd2113e014909f97d1955f12c3/content/browser/renderer_host/media/video_capture_controller.h [modify] https://crrev.com/cf466af46f6bc3bd2113e014909f97d1955f12c3/content/browser/renderer_host/media/video_capture_manager.cc [modify] https://crrev.com/cf466af46f6bc3bd2113e014909f97d1955f12c3/content/browser/renderer_host/media/video_capture_manager.h [modify] https://crrev.com/cf466af46f6bc3bd2113e014909f97d1955f12c3/content/browser/renderer_host/media/video_capture_manager_unittest.cc
,
May 19 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by c.pa...@samsung.com
, Apr 5 2017