|
|
Chromium Code Reviews|
Created:
3 years, 11 months ago by henryhsu Modified:
3 years, 11 months 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. |
DescriptionSort camera device list to use front camera first
Since javascript doesn't have API to get camera facing, we sort the
reported camera device list by facing. Then app can use the front
camera by default.
BUG=684224, chrome-os-partner:61756
TEST=powerwash device and camera app uses the front camera by default.
Review-Url: https://codereview.chromium.org/2648973003
Cr-Commit-Position: refs/heads/master@{#446307}
Committed: https://chromium.googlesource.com/chromium/src/+/e9c70e53e5da9b0065ef7f5c6ce78d5b46e49875
Patch Set 1 #Patch Set 2 : fix issue number #
Total comments: 4
Patch Set 3 : add comments #
Total comments: 2
Patch Set 4 : address comments #Patch Set 5 : add bug number #
Total comments: 2
Patch Set 6 : address comments #
Total comments: 6
Patch Set 7 : address comments #
Total comments: 4
Patch Set 8 : address comment #
Total comments: 7
Patch Set 9 : address comment #
Total comments: 1
Patch Set 10 : remove #if #
Messages
Total messages: 56 (20 generated)
henryhsu@chromium.org changed reviewers: + posciak@chromium.org, shenghao@chromium.org, wuchengli@chromium.org
PTAL
henryhsu@chromium.org changed reviewers: + mcasas@chromium.org
Description was changed from ========== Sort camera device list to use front camera first Since javascript doesn't have API to get camera facing, we sort the reported camera device list by facing. Then app can use the front camera by default. BUG=543997 TEST=powerwash device and camera app uses the front camera by default. ========== to ========== Sort camera device list to use front camera first Since javascript doesn't have API to get camera facing, we sort the reported camera device list by facing. Then app can use the front camera by default. BUG=chrome-os-partner:61756 TEST=powerwash device and camera app uses the front camera by default. ==========
mtomasz@chromium.org changed reviewers: + mtomasz@chromium.org
https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... media/capture/video/linux/video_capture_device_factory_linux.cc:231: std::sort(device_descriptors->begin(), device_descriptors->end()); Does it mean that the default camera in chrome://settings will be set to the facing one (unless it's explicitly changed by user)?
https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... media/capture/video/linux/video_capture_device_factory_linux.cc:231: std::sort(device_descriptors->begin(), device_descriptors->end()); On 2017/01/23 07:15:22, mtomasz wrote: > Does it mean that the default camera in chrome://settings will be set to the > facing one (unless it's explicitly changed by user)? I don't see default camera in chrome://settings. But I believe chrome chooses the first one in the list as the default camera. I tried to powerwash the device and changed the order of the list. I can see the camera app used the first camera of the provided list.
On 2017/01/23 08:04:38, henryhsu wrote: > https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... > File media/capture/video/linux/video_capture_device_factory_linux.cc (right): > > https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... > media/capture/video/linux/video_capture_device_factory_linux.cc:231: > std::sort(device_descriptors->begin(), device_descriptors->end()); > On 2017/01/23 07:15:22, mtomasz wrote: > > Does it mean that the default camera in chrome://settings will be set to the > > facing one (unless it's explicitly changed by user)? > > I don't see default camera in chrome://settings. > But I believe chrome chooses the first one in the list as the default camera. > I tried to powerwash the device and changed the order of the list. > I can see the camera app used the first camera of the provided list. You can set the default Camera in chrome://settings/content, and it's set to something initial from beginning. What version of Camera app did you use? You can check by typing "VER" in the camera app.
On 2017/01/23 08:07:03, mtomasz wrote: > On 2017/01/23 08:04:38, henryhsu wrote: > > > https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... > > File media/capture/video/linux/video_capture_device_factory_linux.cc (right): > > > > > https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... > > media/capture/video/linux/video_capture_device_factory_linux.cc:231: > > std::sort(device_descriptors->begin(), device_descriptors->end()); > > On 2017/01/23 07:15:22, mtomasz wrote: > > > Does it mean that the default camera in chrome://settings will be set to the > > > facing one (unless it's explicitly changed by user)? > > > > I don't see default camera in chrome://settings. > > But I believe chrome chooses the first one in the list as the default camera. > > I tried to powerwash the device and changed the order of the list. > > I can see the camera app used the first camera of the provided list. > > You can set the default Camera in chrome://settings/content, and it's set to > something initial from beginning. What version of Camera app did you use? You > can check by typing "VER" in the camera app. Version 4.4.4 And I just confirmed that the default camera in chrome://settings/content will be set to the first one of the provided list.
mtomasz@chromium.org changed reviewers: - mtomasz@chromium.org
Thanks for confirming!
https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... media/capture/video/linux/video_capture_device_factory_linux.cc:231: std::sort(device_descriptors->begin(), device_descriptors->end()); May you add comment to document the reason we want to sort it?
https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... media/capture/video/linux/video_capture_device_factory_linux.cc:231: std::sort(device_descriptors->begin(), device_descriptors->end()); On 2017/01/23 10:18:21, shenghao wrote: > May you add comment to document the reason we want to sort it? Done.
On 2017/01/23 11:25:20, henryhsu wrote: > https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... > File media/capture/video/linux/video_capture_device_factory_linux.cc (right): > > https://codereview.chromium.org/2648973003/diff/20001/media/capture/video/lin... > media/capture/video/linux/video_capture_device_factory_linux.cc:231: > std::sort(device_descriptors->begin(), device_descriptors->end()); > On 2017/01/23 10:18:21, shenghao wrote: > > May you add comment to document the reason we want to sort it? > > Done. lgtm
mcasas@chromium.org changed reviewers: + hta@chromium.org
The bug BUG=chrome-os-partner:61756 is not publicly accessible, I think, please make a public crbug.com/ or change the permissions. Also, I'm not 100% sure this change would not break existing content. I'd like to get hta@ opinion. https://codereview.chromium.org/2648973003/diff/40001/media/capture/video/lin... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/40001/media/capture/video/lin... media/capture/video/linux/video_capture_device_factory_linux.cc:233: // TODO(henryhsu): remove this after JS API completed. Add here the https://crbug.com/ appropriate number.
Description was changed from ========== Sort camera device list to use front camera first Since javascript doesn't have API to get camera facing, we sort the reported camera device list by facing. Then app can use the front camera by default. BUG=chrome-os-partner:61756 TEST=powerwash device and camera app uses the front camera by default. ========== to ========== Sort camera device list to use front camera first Since javascript doesn't have API to get camera facing, we sort the reported camera device list by facing. Then app can use the front camera by default. BUG=684224,chrome-os-partner:61756 TEST=powerwash device and camera app uses the front camera by default. ==========
https://codereview.chromium.org/2648973003/diff/40001/media/capture/video/lin... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/40001/media/capture/video/lin... media/capture/video/linux/video_capture_device_factory_linux.cc:233: // TODO(henryhsu): remove this after JS API completed. On 2017/01/23 16:46:59, mcasas wrote: > Add here the https://crbug.com/ appropriate number. Done.
https://codereview.chromium.org/2648973003/diff/80001/media/capture/video/vid... File media/capture/video/video_capture_device_descriptor.h (right): https://codereview.chromium.org/2648973003/diff/80001/media/capture/video/vid... media/capture/video/video_capture_device_descriptor.h:70: // Front camera first We should prefer on-device cameras over external cameras. Front > back > none.
https://codereview.chromium.org/2648973003/diff/80001/media/capture/video/vid... File media/capture/video/video_capture_device_descriptor.h (right): https://codereview.chromium.org/2648973003/diff/80001/media/capture/video/vid... media/capture/video/video_capture_device_descriptor.h:70: // Front camera first On 2017/01/24 04:25:09, wuchengli wrote: > We should prefer on-device cameras over external cameras. Front > back > none. Done.
lgtm. hta@ PTAL
Basic structure seems unobjectionable, but I have C++ comments. https://codereview.chromium.org/2648973003/diff/100001/media/capture/video/vi... File media/capture/video/video_capture_device_descriptor.h (right): https://codereview.chromium.org/2648973003/diff/100001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.h:78: } The two-step process here seems strange. Why not initialize the map? ... facing_mapping({{MEDIA_VIDEO_FACING_USER, 1}, {MEDIA_VIDEO_FACING_ENVIRONMENT, 2}, {MEDIA_VIDEO_FACING_NONE, 3}}) https://codereview.chromium.org/2648973003/diff/100001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.h:79: if (facing_mapping.at(facing) < facing_mapping.at(other.facing)) What happens here if facing = MEDIA_VIDEO_FACING_LEFT? This enumeration at JS level has two more values - left and right. Sooner or later these will percolate down to this level too. https://codereview.chromium.org/2648973003/diff/100001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.h:80: return true; I would expect a function this long to get hollered at by presubmit for being "too long for a .cc" We already have video_capture_device_descriptor.cc; suggest to move it there.
Description was changed from ========== Sort camera device list to use front camera first Since javascript doesn't have API to get camera facing, we sort the reported camera device list by facing. Then app can use the front camera by default. BUG=684224,chrome-os-partner:61756 TEST=powerwash device and camera app uses the front camera by default. ========== to ========== Sort camera device list to use front camera first Since javascript doesn't have API to get camera facing, we sort the reported camera device list by facing. Then app can use the front camera by default. BUG=684224,chrome-os-partner:61756 TEST=powerwash device and camera app uses the front camera by default. ==========
hta@chromium.org changed reviewers: + guidou@webrtc.org
Also added guidou as reviewer, since he's currently rewriting code that (among other things) queries the default device.
https://codereview.chromium.org/2648973003/diff/100001/media/capture/video/vi... File media/capture/video/video_capture_device_descriptor.h (right): https://codereview.chromium.org/2648973003/diff/100001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.h:78: } On 2017/01/24 09:46:24, hta - Chromium wrote: > The two-step process here seems strange. Why not initialize the map? > > ... facing_mapping({{MEDIA_VIDEO_FACING_USER, 1}, > {MEDIA_VIDEO_FACING_ENVIRONMENT, 2}, > {MEDIA_VIDEO_FACING_NONE, 3}}) Done. https://codereview.chromium.org/2648973003/diff/100001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.h:79: if (facing_mapping.at(facing) < facing_mapping.at(other.facing)) On 2017/01/24 09:46:24, hta - Chromium wrote: > What happens here if facing = MEDIA_VIDEO_FACING_LEFT? > > This enumeration at JS level has two more values - left and right. Sooner or > later these will percolate down to this level too. It's impossible to have MEDIA_VIDEO_FACING_LEFT value. Because facing is assigned by camera_facing_chromeos.cc not from JS level. Please reference https://codereview.chromium.org/2609863004/patch/400001/410012 https://codereview.chromium.org/2648973003/diff/100001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.h:80: return true; On 2017/01/24 09:46:24, hta - Chromium wrote: > I would expect a function this long to get hollered at by presubmit for being > "too long for a .cc" > > We already have video_capture_device_descriptor.cc; suggest to move it there. Done.
guidou@chromium.org changed reviewers: + guidou@chromium.org
https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/li... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/li... media/capture/video/linux/video_capture_device_factory_linux.cc:234: std::sort(device_descriptors->begin(), device_descriptors->end()); Is this sorting relevant only for linux, or should it apply to other platforms as well? https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/vi... File media/capture/video/video_capture_device_descriptor.cc (right): https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.cc:50: std::map<media::VideoFacingMode, size_t> facing_mapping{ nit: Optional. the map is fine, but maybe you can also use an array: int facing_mapping[NUM_MEDIA_VIDEO_FACING_MODE]; facing_mapping[MEDIA_VIDEO_FACING_USER] = 0; facing_mapping[MEDIA_VIDEO_FACING_ENVIRONMENT] = 1; facing_mapping[MEDIA_VIDEO_FACING_NONE] = 2;
https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/li... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/li... media/capture/video/linux/video_capture_device_factory_linux.cc:234: std::sort(device_descriptors->begin(), device_descriptors->end()); On 2017/01/24 10:42:07, Guido Urdaneta wrote: > Is this sorting relevant only for linux, or should it apply to other platforms > as well? This is for chromebook https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/vi... File media/capture/video/video_capture_device_descriptor.cc (right): https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.cc:50: std::map<media::VideoFacingMode, size_t> facing_mapping{ On 2017/01/24 10:42:07, Guido Urdaneta wrote: > nit: Optional. the map is fine, but maybe you can also use an array: > int facing_mapping[NUM_MEDIA_VIDEO_FACING_MODE]; > facing_mapping[MEDIA_VIDEO_FACING_USER] = 0; > facing_mapping[MEDIA_VIDEO_FACING_ENVIRONMENT] = 1; > facing_mapping[MEDIA_VIDEO_FACING_NONE] = 2; > Done.
On 2017/01/25 03:28:18, henryhsu wrote: > https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/li... > File media/capture/video/linux/video_capture_device_factory_linux.cc (right): > > https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/li... > media/capture/video/linux/video_capture_device_factory_linux.cc:234: > std::sort(device_descriptors->begin(), device_descriptors->end()); > On 2017/01/24 10:42:07, Guido Urdaneta wrote: > > Is this sorting relevant only for linux, or should it apply to other platforms > > as well? > > This is for chromebook > > https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/vi... > File media/capture/video/video_capture_device_descriptor.cc (right): > > https://codereview.chromium.org/2648973003/diff/120001/media/capture/video/vi... > media/capture/video/video_capture_device_descriptor.cc:50: > std::map<media::VideoFacingMode, size_t> facing_mapping{ > On 2017/01/24 10:42:07, Guido Urdaneta wrote: > > nit: Optional. the map is fine, but maybe you can also use an array: > > int facing_mapping[NUM_MEDIA_VIDEO_FACING_MODE]; > > facing_mapping[MEDIA_VIDEO_FACING_USER] = 0; > > facing_mapping[MEDIA_VIDEO_FACING_ENVIRONMENT] = 1; > > facing_mapping[MEDIA_VIDEO_FACING_NONE] = 2; > > > > Done. Hi, PTAL. We have to land this by end of Thursday.
lgtm, but take a look at the question below and adjust the TODO comment if necessary. https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/li... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/li... media/capture/video/linux/video_capture_device_factory_linux.cc:234: // TODO(henryhsu): remove this after JS API completed (crbug.com/543997). Is this comment valid? It is true that once crbug.com/543997 is fixed, constraints can be used to specify which camera to use based on facing mode. However, what about the case when no constraint is specified so that all cameras are equally valid? In this case the implementation needs a tie-breaking rule. Would you prefer the tie breaker to choose front cameras over other cameras? If so, the tie breaker could be implemented by preferring cameras that appear first in the enumeration, so this ordering might need to be kept even after crbug.com/543997.
https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/vi... File media/capture/video/video_capture_device_descriptor.cc (right): https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.cc:48: int facing_mapping[NUM_MEDIA_VIDEO_FACING_MODE] = {0}; nit: I don't think you need the {0} initialization, but up to you.
https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/li... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/li... media/capture/video/linux/video_capture_device_factory_linux.cc:234: // TODO(henryhsu): remove this after JS API completed (crbug.com/543997). On 2017/01/25 10:24:47, Guido Urdaneta wrote: > Is this comment valid? > It is true that once crbug.com/543997 is fixed, constraints can be used to > specify which camera to use based on facing mode. > However, what about the case when no constraint is specified so that all cameras > are equally valid? > In this case the implementation needs a tie-breaking rule. > Would you prefer the tie breaker to choose front cameras over other cameras? If > so, the tie breaker could be implemented by preferring cameras that appear first > in the enumeration, so this ordering might need to be kept even after > crbug.com/543997. Once crbug.com/543997 is fixed, caller should have all information to decide which camera should be opened. The preference logic should be moved to caller (camera app). So all cameras here are equal.
hta@, PTAL. We need owner approval. Thank you.
Lgtm 25. jan. 2017 11:46 skrev <henryhsu@chromium.org>: > hta@, PTAL. We need owner approval. Thank you. > > https://codereview.chromium.org/2648973003/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
lgtm with suggestion. https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/li... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/li... media/capture/video/linux/video_capture_device_factory_linux.cc:233: // apps can use the front camera by default. nit: s/to make apps can use/to make sure apps use/ ? https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/vi... File media/capture/video/video_capture_device_descriptor.cc (right): https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.cc:51: facing_mapping[MEDIA_VIDEO_FACING_NONE] = 0; I'd do: static const int kFacingMapping[] = {0, 1, 2}; static_assert(kFacingMapping(MEDIA_VIDEO_FACING_NONE == 0, "None"); static_assert(kFacingMapping(MEDIA_VIDEO_FACING_ENVIRONMENT == 1, "blabla"); static_assert(kFacingMapping(MEDIA_VIDEO_FACING_USER == 2, "bla"); so the correspondence would be enforced at compile time, and the penalty of initializing the array is taken once.
https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/li... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/li... media/capture/video/linux/video_capture_device_factory_linux.cc:233: // apps can use the front camera by default. On 2017/01/25 17:35:15, mcasas wrote: > nit: s/to make apps can use/to make sure apps use/ ? Done. https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/vi... File media/capture/video/video_capture_device_descriptor.cc (right): https://codereview.chromium.org/2648973003/diff/140001/media/capture/video/vi... media/capture/video/video_capture_device_descriptor.cc:51: facing_mapping[MEDIA_VIDEO_FACING_NONE] = 0; On 2017/01/25 17:35:15, mcasas wrote: > I'd do: > > static const int kFacingMapping[] = {0, 1, 2}; > static_assert(kFacingMapping(MEDIA_VIDEO_FACING_NONE == 0, "None"); > static_assert(kFacingMapping(MEDIA_VIDEO_FACING_ENVIRONMENT == 1, "blabla"); > static_assert(kFacingMapping(MEDIA_VIDEO_FACING_USER == 2, "bla"); > > so the correspondence would be enforced at compile time, > and the penalty of initializing the array is taken once. Done.
The CQ bit was checked by henryhsu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from shenghao@chromium.org, wuchengli@chromium.org, guidou@chromium.org, mcasas@chromium.org Link to the patchset: https://codereview.chromium.org/2648973003/#ps160001 (title: "address comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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_androi...)
lgtm wouldn't mind seeing this happen on all platforms, permanently. It's within the remit of the platform to choose to do it like this. https://codereview.chromium.org/2648973003/diff/160001/media/capture/video/li... File media/capture/video/linux/video_capture_device_factory_linux.cc (right): https://codereview.chromium.org/2648973003/diff/160001/media/capture/video/li... media/capture/video/linux/video_capture_device_factory_linux.cc:231: #if defined(OS_CHROMEOS) Note: I'd remove the #if. Consistent, testable cross-platform behavior is important.
The CQ bit was checked by shenghao@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by henryhsu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from shenghao@chromium.org, guidou@chromium.org, mcasas@chromium.org, wuchengli@chromium.org, hta@chromium.org Link to the patchset: https://codereview.chromium.org/2648973003/#ps180001 (title: "remove #if")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by henryhsu@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 180001, "attempt_start_ts": 1485431297496680,
"parent_rev": "d91883069c9f837b518a88e208354543cf4182d5", "commit_rev":
"e9c70e53e5da9b0065ef7f5c6ce78d5b46e49875"}
Message was sent while issue was closed.
Description was changed from ========== Sort camera device list to use front camera first Since javascript doesn't have API to get camera facing, we sort the reported camera device list by facing. Then app can use the front camera by default. BUG=684224,chrome-os-partner:61756 TEST=powerwash device and camera app uses the front camera by default. ========== to ========== Sort camera device list to use front camera first Since javascript doesn't have API to get camera facing, we sort the reported camera device list by facing. Then app can use the front camera by default. BUG=684224,chrome-os-partner:61756 TEST=powerwash device and camera app uses the front camera by default. Review-Url: https://codereview.chromium.org/2648973003 Cr-Commit-Position: refs/heads/master@{#446307} Committed: https://chromium.googlesource.com/chromium/src/+/e9c70e53e5da9b0065ef7f5c6ce7... ==========
Message was sent while issue was closed.
Committed patchset #10 (id:180001) as https://chromium.googlesource.com/chromium/src/+/e9c70e53e5da9b0065ef7f5c6ce7... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
