|
|
Created:
4 years, 3 months ago by sakal Modified:
4 years, 3 months ago Reviewers:
magjed_webrtc CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdd statistics for the time it takes to start and stop the camera on Camera2.
BUG=webrtc:6302
Committed: https://crrev.com/1a0533dc3ddd41b9bac9d2fd39f1abfbd0343699
Cr-Commit-Position: refs/heads/master@{#14180}
Patch Set 1 #Patch Set 2 : Rebase #
Total comments: 4
Patch Set 3 : Change last addSampleLogged to addSample. #Patch Set 4 : Nicer names for stats. #
Total comments: 2
Patch Set 5 : Address nit. #Messages
Total messages: 26 (15 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
PTAL
Description was changed from ========== Add statistics for the time it takes to start and stop the camera on Camera2. BUG=webrtc:6302 ========== to ========== Add statistics for the time it takes to start and stop the camera on Camera2. BUG=webrtc:6302 ==========
sakal@webrtc.org changed reviewers: + magjed@google.com
PTAL
sakal@webrtc.org changed reviewers: + magjed@webrtc.org - magjed@google.com
https://codereview.webrtc.org/2326483003/diff/60001/webrtc/api/android/java/s... File webrtc/api/android/java/src/org/webrtc/Camera2Session.java (right): https://codereview.webrtc.org/2326483003/diff/60001/webrtc/api/android/java/s... webrtc/api/android/java/src/org/webrtc/Camera2Session.java:195: int startTimeMs = (int) ((System.nanoTime() - constructionTimeNs) / 1000000L); Maybe System.currentTimeMillis() would be more suitable? https://codereview.webrtc.org/2326483003/diff/60001/webrtc/api/android/java/s... webrtc/api/android/java/src/org/webrtc/Camera2Session.java:196: camera2StartTimeMsHistogram.addSampleLogged(startTimeMs); Change to no log ;)
https://codereview.webrtc.org/2326483003/diff/60001/webrtc/api/android/java/s... File webrtc/api/android/java/src/org/webrtc/Camera2Session.java (right): https://codereview.webrtc.org/2326483003/diff/60001/webrtc/api/android/java/s... webrtc/api/android/java/src/org/webrtc/Camera2Session.java:195: int startTimeMs = (int) ((System.nanoTime() - constructionTimeNs) / 1000000L); On 2016/09/08 14:53:31, magjed_webrtc wrote: > Maybe System.currentTimeMillis() would be more suitable? System.currentTimeMillis() might not be accurate to a millisecond level. currentTimeMillis() gives a guarantee of it being milliseconds since January 1, 1970 UTC. nanoTime() doesn't make that guarantee and can be more accurate. https://codereview.webrtc.org/2326483003/diff/60001/webrtc/api/android/java/s... webrtc/api/android/java/src/org/webrtc/Camera2Session.java:196: camera2StartTimeMsHistogram.addSampleLogged(startTimeMs); On 2016/09/08 14:53:31, magjed_webrtc wrote: > Change to no log ;) Done.
The CQ bit was checked by sakal@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/16409)
lgtm https://codereview.webrtc.org/2326483003/diff/100001/webrtc/api/android/java/... File webrtc/api/android/java/src/org/webrtc/Camera2Session.java (right): https://codereview.webrtc.org/2326483003/diff/100001/webrtc/api/android/java/... webrtc/api/android/java/src/org/webrtc/Camera2Session.java:195: int startTimeMs = (int) ((System.nanoTime() - constructionTimeNs) / 1000000L); nit: Can we use TimeUnit.NANOSECONDS.toMillis() instead of 1000000L?
https://codereview.webrtc.org/2326483003/diff/100001/webrtc/api/android/java/... File webrtc/api/android/java/src/org/webrtc/Camera2Session.java (right): https://codereview.webrtc.org/2326483003/diff/100001/webrtc/api/android/java/... webrtc/api/android/java/src/org/webrtc/Camera2Session.java:195: int startTimeMs = (int) ((System.nanoTime() - constructionTimeNs) / 1000000L); On 2016/09/09 10:24:36, magjed_webrtc wrote: > nit: Can we use TimeUnit.NANOSECONDS.toMillis() instead of 1000000L? Done.
The CQ bit was checked by sakal@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org Link to the patchset: https://codereview.webrtc.org/2326483003/#ps120001 (title: "Address nit.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by sakal@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== Add statistics for the time it takes to start and stop the camera on Camera2. BUG=webrtc:6302 ========== to ========== Add statistics for the time it takes to start and stop the camera on Camera2. BUG=webrtc:6302 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:120001)
Message was sent while issue was closed.
Description was changed from ========== Add statistics for the time it takes to start and stop the camera on Camera2. BUG=webrtc:6302 ========== to ========== Add statistics for the time it takes to start and stop the camera on Camera2. BUG=webrtc:6302 Committed: https://crrev.com/1a0533dc3ddd41b9bac9d2fd39f1abfbd0343699 Cr-Commit-Position: refs/heads/master@{#14180} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/1a0533dc3ddd41b9bac9d2fd39f1abfbd0343699 Cr-Commit-Position: refs/heads/master@{#14180} |