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

Issue 602063002: Android Video Capture: Removed references to Android.Hardware.Camera from the factory and cleanup (Closed)

Created:
6 years, 3 months ago by mcasas
Modified:
6 years, 2 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org, avayvod+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Android Video Capture: Removed references to Android.Hardware.Camera from the factory and cleanup As part of the masterplan of the bug, this CL removes the explicit android.hardware.Camera classes manipulations, asking instead classes VideoCapture{,Android,Tango} for this info. Some other minor cleanup goes along with it: VideoCaptureFactory.CamParams is only used inside VideoCaptureTango and to retrieve the name of the special Tango camera, that's refactored to hide the CamParams inside VideoCaptureTango. Other methods are further simplified in the VideoCaptureFactory so their use from C++ is more evident. BUG=418052 Committed: https://crrev.com/928fb4d626d5093ca339e3db63edd5143ac3c678 Cr-Commit-Position: refs/heads/master@{#297380}

Patch Set 1 : #

Patch Set 2 : self reviewed #

Patch Set 3 : Rebasing. Large VideoCaptureFactory cleanup. #

Patch Set 4 : Rebase and removed redundant import. #

Total comments: 6

Patch Set 5 : qinmin@ nit and tiny style. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+93 lines, -120 lines) Patch
M media/base/android/java/src/org/chromium/media/VideoCapture.java View 1 2 3 chunks +13 lines, -11 lines 0 comments Download
M media/base/android/java/src/org/chromium/media/VideoCaptureAndroid.java View 1 2 1 chunk +10 lines, -0 lines 0 comments Download
M media/base/android/java/src/org/chromium/media/VideoCaptureFactory.java View 1 2 3 4 3 chunks +30 lines, -86 lines 0 comments Download
M media/base/android/java/src/org/chromium/media/VideoCaptureTango.java View 1 2 3 4 5 chunks +34 lines, -13 lines 0 comments Download
M media/video/capture/android/video_capture_device_factory_android.cc View 1 2 1 chunk +6 lines, -10 lines 0 comments Download

Messages

Total messages: 20 (7 generated)
mcasas
magjed@: PTAL (PLZ note that there's a slightly different coding style for J and that ...
6 years, 3 months ago (2014-09-25 10:32:34 UTC) #4
magjed_chromium
Why do we want to remove references to Android.Hardware.Camera from the factory? We still have ...
6 years, 3 months ago (2014-09-25 10:51:27 UTC) #5
magjed_chromium
after some offline discussion, lgtm
6 years, 2 months ago (2014-09-26 08:41:49 UTC) #6
mcasas
magjed@ please have another look.
6 years, 2 months ago (2014-09-29 09:34:49 UTC) #9
magjed_chromium
lgtm
6 years, 2 months ago (2014-09-29 09:42:22 UTC) #10
mcasas
perkj@: media/video/capture/android/video_capture_device_factory_android.cc qinmin@: media/base/android/java/src/org/chromium/media/VideoCapture*.java PTAL
6 years, 2 months ago (2014-09-29 09:44:14 UTC) #12
perkj_chrome
c++ lgtm https://codereview.chromium.org/602063002/diff/140001/media/base/android/java/src/org/chromium/media/VideoCapture.java File media/base/android/java/src/org/chromium/media/VideoCapture.java (right): https://codereview.chromium.org/602063002/diff/140001/media/base/android/java/src/org/chromium/media/VideoCapture.java#newcode82 media/base/android/java/src/org/chromium/media/VideoCapture.java:82: new android.hardware.Camera.CameraInfo(); indentation ? https://codereview.chromium.org/602063002/diff/140001/media/base/android/java/src/org/chromium/media/VideoCapture.java#newcode111 media/base/android/java/src/org/chromium/media/VideoCapture.java:111: android.hardware.Camera.CameraInfo ...
6 years, 2 months ago (2014-09-29 09:59:17 UTC) #13
mcasas
https://codereview.chromium.org/602063002/diff/140001/media/base/android/java/src/org/chromium/media/VideoCapture.java File media/base/android/java/src/org/chromium/media/VideoCapture.java (right): https://codereview.chromium.org/602063002/diff/140001/media/base/android/java/src/org/chromium/media/VideoCapture.java#newcode82 media/base/android/java/src/org/chromium/media/VideoCapture.java:82: new android.hardware.Camera.CameraInfo(); On 2014/09/29 09:59:17, perkj wrote: > indentation ...
6 years, 2 months ago (2014-09-29 10:03:50 UTC) #14
qinmin
lgtm % nit https://codereview.chromium.org/602063002/diff/140001/media/base/android/java/src/org/chromium/media/VideoCaptureFactory.java File media/base/android/java/src/org/chromium/media/VideoCaptureFactory.java (right): https://codereview.chromium.org/602063002/diff/140001/media/base/android/java/src/org/chromium/media/VideoCaptureFactory.java#newcode77 media/base/android/java/src/org/chromium/media/VideoCaptureFactory.java:77: if (!isSpecialDevice()) nit: {} needed
6 years, 2 months ago (2014-09-29 18:47:37 UTC) #15
mcasas
https://codereview.chromium.org/602063002/diff/140001/media/base/android/java/src/org/chromium/media/VideoCaptureFactory.java File media/base/android/java/src/org/chromium/media/VideoCaptureFactory.java (right): https://codereview.chromium.org/602063002/diff/140001/media/base/android/java/src/org/chromium/media/VideoCaptureFactory.java#newcode77 media/base/android/java/src/org/chromium/media/VideoCaptureFactory.java:77: if (!isSpecialDevice()) On 2014/09/29 18:47:37, qinmin wrote: > nit: ...
6 years, 2 months ago (2014-09-30 07:29:24 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/602063002/160001
6 years, 2 months ago (2014-09-30 07:29:59 UTC) #18
commit-bot: I haz the power
Committed patchset #5 (id:160001) as 85b1492e65a6c6480b33dd862c16d32211d203d4
6 years, 2 months ago (2014-09-30 08:21:45 UTC) #19
commit-bot: I haz the power
6 years, 2 months ago (2014-09-30 08:22:32 UTC) #20
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/928fb4d626d5093ca339e3db63edd5143ac3c678
Cr-Commit-Position: refs/heads/master@{#297380}

Powered by Google App Engine
This is Rietveld 408576698