Skip to content

Commit

Permalink
CCA: Refactor camera info retrieval
Browse files Browse the repository at this point in the history 
Since now we decouple CameraAppDevice and VCD, it does not make sense to
query and bind the camera info to CameraAppDevice while constructing it.

Also, since camera info does not change during a camera session, caching
it in CCA to avoid copying the whole static camera metadata every time
we need it in CCA.

Therefore, this CL refactors the way how CCA/CameraAppDevice retrieves
the camera info and adding mechanism to make the camera info updated
when the underlying camera is restarted.

Bug: b:204737149
Test: Manually
Change-Id: I4560ecca1682ef82373862bdeca6f603a3d1aaa7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3314414
Reviewed-by: Pi-Hsun Shih <pihsun@chromium.org>
Commit-Queue: Wei Lee <wtlee@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1104872}
  • Loading branch information
Wei Lee authored and Chromium LUCI CQ committed Feb 14, 2023
1 parent b9d5483 commit c023c61
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 59 deletions.
40 changes: 31 additions & 9 deletions ash/webui/camera_app_ui/resources/js/mojo/device_operator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
CameraEventObserverCallbackRouter,
CameraFacing,
CameraInfo,
CameraInfoObserverCallbackRouter,
CameraMetadata,
CameraMetadataEntry,
CameraMetadataTag,
Expand Down Expand Up @@ -145,7 +146,7 @@ export class DeviceOperator {
/**
* Map for cached camera infos.
*/
private readonly cameraInfos = new Map<string, Promise<CameraInfo>>();
private readonly cameraInfos = new Map<string, CancelableEvent<CameraInfo>>();

/**
* Return if the direct communication between camera app and video capture
Expand All @@ -161,6 +162,11 @@ export class DeviceOperator {
*/
removeDevice(deviceId: string): void {
this.devices.delete(deviceId);

const info = this.cameraInfos.get(deviceId);
if (info !== undefined) {
info.signalError(new Error('Camera info retrieval is canceled'));
}
this.cameraInfos.delete(deviceId);
}

Expand Down Expand Up @@ -202,15 +208,12 @@ export class DeviceOperator {
private async getCameraInfo(deviceId: string): Promise<CameraInfo> {
const info = this.cameraInfos.get(deviceId);
if (info !== undefined) {
return info;
return info.wait();
}
const newInfo = (async () => {
const device = await this.getDevice(deviceId);
const {cameraInfo} = await device.getCameraInfo();
return cameraInfo;
})();
this.cameraInfos.set(deviceId, newInfo);
return newInfo;
const onInfoReady = new CancelableEvent<CameraInfo>();
this.cameraInfos.set(deviceId, onInfoReady);
this.registerCameraInfoObserver(deviceId);
return onInfoReady.wait();
}

/**
Expand Down Expand Up @@ -710,6 +713,25 @@ export class DeviceOperator {
return observerCallbackRouter;
}

/**
* Registers observer to monitor when the camera info is updated.
*
* @param deviceId The id of the target camera device.
*/
async registerCameraInfoObserver(deviceId: string): Promise<void> {
const observerCallbackRouter =
wrapEndpoint(new CameraInfoObserverCallbackRouter());
observerCallbackRouter.onCameraInfoUpdated.addListener(
(info: CameraInfo) => {
const onInfoReady = this.cameraInfos.get(deviceId);
assert(onInfoReady !== undefined);
onInfoReady.signal(info);
});
const device = await this.getDevice(deviceId);
await device.registerCameraInfoObserver(
observerCallbackRouter.$.bindNewPipeAndPassRemote());
}

/**
* Returns whether the blob video snapshot feature is enabled on the device.
*
Expand Down
1 change: 1 addition & 0 deletions ash/webui/camera_app_ui/resources/js/mojo/type.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export {
CameraAppDeviceProviderRemote,
CameraAppDeviceRemote,
CameraEventObserverCallbackRouter,
CameraInfoObserverCallbackRouter,
CaptureIntent,
DocumentCornersObserverCallbackRouter,
Effect,
Expand Down
74 changes: 44 additions & 30 deletions media/capture/video/chromeos/camera_app_device_bridge_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,15 @@ void CameraAppDeviceBridgeImpl::BindReceiver(
void CameraAppDeviceBridgeImpl::OnVideoCaptureDeviceCreated(
const std::string& device_id,
scoped_refptr<base::SingleThreadTaskRunner> ipc_task_runner) {
base::AutoLock lock(task_runner_map_lock_);
DCHECK_EQ(ipc_task_runners_.count(device_id), 0u);
ipc_task_runners_.emplace(device_id, ipc_task_runner);
{
base::AutoLock lock(task_runner_map_lock_);
DCHECK_EQ(ipc_task_runners_.count(device_id), 0u);
ipc_task_runners_.emplace(device_id, ipc_task_runner);
}

// Update the cached camera info while VCD is connected as well so that when
// the camera service is restarted the camera info can be updated properly.
UpdateCameraInfo(device_id);
}

void CameraAppDeviceBridgeImpl::OnVideoCaptureDeviceClosing(
Expand Down Expand Up @@ -91,6 +97,24 @@ void CameraAppDeviceBridgeImpl::OnDeviceMojoDisconnected(
std::move(remove_device).Run();
}

void CameraAppDeviceBridgeImpl::UpdateCameraInfo(const std::string& device_id) {
cros::mojom::CameraInfoPtr camera_info;
{
base::AutoLock lock(camera_info_getter_lock_);
DCHECK(camera_info_getter_);
camera_info = camera_info_getter_.Run(device_id);
}

{
base::AutoLock lock(device_map_lock_);
auto it = camera_app_devices_.find(device_id);
if (it != camera_app_devices_.end()) {
const auto& device = it->second;
device->OnCameraInfoUpdated(std::move(camera_info));
}
}
}

void CameraAppDeviceBridgeImpl::InvalidateDevicePtrsOnDeviceIpcThread(
const std::string& device_id,
bool should_disable_new_ptrs,
Expand Down Expand Up @@ -159,37 +183,27 @@ void CameraAppDeviceBridgeImpl::GetCameraAppDevice(
DCHECK(is_supported_);

mojo::PendingRemote<cros::mojom::CameraAppDevice> device_remote;
auto* device = GetOrCreateCameraAppDevice(device_id);
DCHECK(device);

device->BindReceiver(device_remote.InitWithNewPipeAndPassReceiver());
{
base::AutoLock lock(device_map_lock_);

CameraAppDeviceImpl* device;
auto it = camera_app_devices_.find(device_id);
if (it != camera_app_devices_.end()) {
device = it->second.get();
} else {
auto device_impl =
std::make_unique<media::CameraAppDeviceImpl>(device_id);
const auto& iterator =
camera_app_devices_.emplace(device_id, std::move(device_impl)).first;
device = iterator->second.get();
}
device->BindReceiver(device_remote.InitWithNewPipeAndPassReceiver());
}
UpdateCameraInfo(device_id);
std::move(callback).Run(cros::mojom::GetCameraAppDeviceStatus::SUCCESS,
std::move(device_remote));
}

media::CameraAppDeviceImpl*
CameraAppDeviceBridgeImpl::GetOrCreateCameraAppDevice(
const std::string& device_id) {
base::AutoLock lock(device_map_lock_);
auto it = camera_app_devices_.find(device_id);
if (it != camera_app_devices_.end()) {
return it->second.get();
}

base::AutoLock camera_info_lock(camera_info_getter_lock_);
// Since we ensure that VideoCaptureDeviceFactory is created before binding
// CameraAppDeviceBridge and VideoCaptureDeviceFactory is only destroyed when
// the video capture service dies, we can guarantee that |camera_info_getter_|
// is always valid here.
DCHECK(camera_info_getter_);

auto device_info = camera_info_getter_.Run(device_id);
auto device_impl = std::make_unique<media::CameraAppDeviceImpl>(
device_id, std::move(device_info));
auto result = camera_app_devices_.emplace(device_id, std::move(device_impl));
return result.first->second.get();
}

void CameraAppDeviceBridgeImpl::IsSupported(IsSupportedCallback callback) {
std::move(callback).Run(is_supported_);
}
Expand Down
4 changes: 2 additions & 2 deletions media/capture/video/chromeos/camera_app_device_bridge_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class CAPTURE_EXPORT CameraAppDeviceBridgeImpl

void OnDeviceMojoDisconnected(const std::string& device_id);

void UpdateCameraInfo(const std::string& device_id);

void InvalidateDevicePtrsOnDeviceIpcThread(const std::string& device_id,
bool should_disable_new_ptrs,
base::OnceClosure callback);
Expand Down Expand Up @@ -82,8 +84,6 @@ class CAPTURE_EXPORT CameraAppDeviceBridgeImpl
private:
friend struct base::DefaultSingletonTraits<CameraAppDeviceBridgeImpl>;

CameraAppDeviceImpl* GetOrCreateCameraAppDevice(const std::string& device_id);

bool is_supported_;

base::Lock camera_info_getter_lock_;
Expand Down
54 changes: 44 additions & 10 deletions media/capture/video/chromeos/camera_app_device_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,9 @@ ReprocessTaskQueue CameraAppDeviceImpl::GetSingleShotReprocessOptions(
return result_task_queue;
}

CameraAppDeviceImpl::CameraAppDeviceImpl(const std::string& device_id,
cros::mojom::CameraInfoPtr camera_info)
CameraAppDeviceImpl::CameraAppDeviceImpl(const std::string& device_id)
: device_id_(device_id),
allow_new_ipc_weak_ptrs_(true),
camera_info_(std::move(camera_info)),
capture_intent_(cros::mojom::CaptureIntent::DEFAULT),
camera_device_context_(nullptr) {}

Expand Down Expand Up @@ -168,6 +166,20 @@ void CameraAppDeviceImpl::OnShutterDone() {
weak_ptr_factory_for_mojo_.GetWeakPtr()));
}

void CameraAppDeviceImpl::OnCameraInfoUpdated(
cros::mojom::CameraInfoPtr camera_info) {
base::AutoLock lock(camera_info_lock_);
camera_info_ = std::move(camera_info);

if (!mojo_task_runner_) {
return;
}
mojo_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&CameraAppDeviceImpl::NotifyCameraInfoUpdatedOnMojoThread,
weak_ptr_factory_for_mojo_.GetWeakPtr()));
}

void CameraAppDeviceImpl::SetCameraDeviceContext(
CameraDeviceContext* camera_device_context) {
base::AutoLock lock(camera_device_context_lock_);
Expand Down Expand Up @@ -198,13 +210,6 @@ bool CameraAppDeviceImpl::IsMultipleStreamsEnabled() {
return multi_stream_enabled_;
}

void CameraAppDeviceImpl::GetCameraInfo(GetCameraInfoCallback callback) {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());
DCHECK(camera_info_);

std::move(callback).Run(camera_info_.Clone());
}

void CameraAppDeviceImpl::SetReprocessOptions(
const std::vector<cros::mojom::Effect>& effects,
mojo::PendingRemote<cros::mojom::ReprocessResultListener> listener,
Expand Down Expand Up @@ -239,6 +244,12 @@ void CameraAppDeviceImpl::SetFpsRange(const gfx::Range& fps_range,

const int entry_length = 2;

base::AutoLock camera_info_lock(camera_info_lock_);
if (!camera_info_) {
LOG(ERROR) << "Camera info is still not available at this moment";
std::move(callback).Run(false);
return;
}
auto& static_metadata = camera_info_->static_camera_characteristics;
auto available_fps_range_entries = GetMetadataEntryAsSpan<int32_t>(
static_metadata, cros::mojom::CameraMetadataTag::
Expand Down Expand Up @@ -366,6 +377,17 @@ void CameraAppDeviceImpl::SetMultipleStreamsEnabled(
std::move(callback).Run();
}

void CameraAppDeviceImpl::RegisterCameraInfoObserver(
mojo::PendingRemote<cros::mojom::CameraInfoObserver> observer,
RegisterCameraInfoObserverCallback callback) {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());

camera_info_observers_.Add(std::move(observer));
std::move(callback).Run();

NotifyCameraInfoUpdatedOnMojoThread();
}

// static
void CameraAppDeviceImpl::DisableEeNr(ReprocessTask* task) {
auto ee_entry =
Expand Down Expand Up @@ -510,4 +532,16 @@ void CameraAppDeviceImpl::NotifyResultMetadataOnMojoThread(
}
}

void CameraAppDeviceImpl::NotifyCameraInfoUpdatedOnMojoThread() {
DCHECK(mojo_task_runner_->BelongsToCurrentThread());

base::AutoLock lock(camera_info_lock_);
if (!camera_info_) {
return;
}
for (auto& observer : camera_info_observers_) {
observer->OnCameraInfoUpdated(camera_info_.Clone());
}
}

} // namespace media
16 changes: 12 additions & 4 deletions media/capture/video/chromeos/camera_app_device_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ class CAPTURE_EXPORT CameraAppDeviceImpl : public cros::mojom::CameraAppDevice {
static ReprocessTaskQueue GetSingleShotReprocessOptions(
media::mojom::ImageCapture::TakePhotoCallback take_photo_callback);

CameraAppDeviceImpl(const std::string& device_id,
cros::mojom::CameraInfoPtr camera_info);
explicit CameraAppDeviceImpl(const std::string& device_id);

CameraAppDeviceImpl(const CameraAppDeviceImpl&) = delete;
CameraAppDeviceImpl& operator=(const CameraAppDeviceImpl&) = delete;
Expand Down Expand Up @@ -121,6 +120,9 @@ class CAPTURE_EXPORT CameraAppDeviceImpl : public cros::mojom::CameraAppDevice {
// Notifies the camera event observers that the shutter is finished.
void OnShutterDone();

// Notifies the camera info observers that the camera info is updated.
void OnCameraInfoUpdated(cros::mojom::CameraInfoPtr camera_info);

// Sets the pointer to the camera device context instance associated with the
// opened camera. Used to configure and query camera frame rotation.
void SetCameraDeviceContext(CameraDeviceContext* device_context);
Expand All @@ -133,7 +135,6 @@ class CAPTURE_EXPORT CameraAppDeviceImpl : public cros::mojom::CameraAppDevice {
bool IsMultipleStreamsEnabled();

// cros::mojom::CameraAppDevice implementations.
void GetCameraInfo(GetCameraInfoCallback callback) override;
void SetReprocessOptions(
const std::vector<cros::mojom::Effect>& effects,
mojo::PendingRemote<cros::mojom::ReprocessResultListener> listener,
Expand Down Expand Up @@ -162,6 +163,9 @@ class CAPTURE_EXPORT CameraAppDeviceImpl : public cros::mojom::CameraAppDevice {
void SetMultipleStreamsEnabled(
bool enabled,
SetMultipleStreamsEnabledCallback callback) override;
void RegisterCameraInfoObserver(
mojo::PendingRemote<cros::mojom::CameraInfoObserver> observer,
RegisterCameraInfoObserverCallback callback) override;

private:
static void DisableEeNr(ReprocessTask* task);
Expand All @@ -186,6 +190,7 @@ class CAPTURE_EXPORT CameraAppDeviceImpl : public cros::mojom::CameraAppDevice {
void NotifyShutterDoneOnMojoThread();
void NotifyResultMetadataOnMojoThread(cros::mojom::CameraMetadataPtr metadata,
cros::mojom::StreamType streamType);
void NotifyCameraInfoUpdatedOnMojoThread();

std::string device_id_;

Expand All @@ -195,7 +200,8 @@ class CAPTURE_EXPORT CameraAppDeviceImpl : public cros::mojom::CameraAppDevice {

mojo::ReceiverSet<cros::mojom::CameraAppDevice> receivers_;

cros::mojom::CameraInfoPtr camera_info_;
base::Lock camera_info_lock_;
cros::mojom::CameraInfoPtr camera_info_ GUARDED_BY(camera_info_lock_);

// It is used for calls which should run on the mojo thread.
scoped_refptr<base::SingleThreadTaskRunner> mojo_task_runner_;
Expand Down Expand Up @@ -237,6 +243,8 @@ class CAPTURE_EXPORT CameraAppDeviceImpl : public cros::mojom::CameraAppDevice {
bool has_ongoing_document_detection_task_ = false;
std::unique_ptr<base::ElapsedTimer> document_detection_timer_ = nullptr;

mojo::RemoteSet<cros::mojom::CameraInfoObserver> camera_info_observers_;

// Client to connect to document detection service. It should only be
// used/destructed on the Mojo thread.
std::unique_ptr<ash::DocumentScannerServiceClient> document_scanner_service_;
Expand Down
15 changes: 11 additions & 4 deletions media/capture/video/chromeos/mojom/camera_app.mojom
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ interface CameraAppDeviceBridge {
// a renderer process, with all the code resides in the Chrome OS rootfs. The
// camera device runs inside the browser process.
interface CameraAppDevice {
// Gets camera information |camera_info| which includes camera facing,
// characteristics, orientation, etc.
GetCameraInfo() => (CameraInfo camera_info);

// Sets reprocess options to bind with the coming take photo request. When
// this method is called, the reprocess options will be queued. All reprocess
// options in the queue will be consumed when ImageCapture::TakePhoto() is
Expand Down Expand Up @@ -140,6 +136,10 @@ interface CameraAppDevice {
// Enable/Disable the multiple streams feature for video recording on the
// device given by its |device_id|.
SetMultipleStreamsEnabled(bool enabled) => ();

// Registers the observer to monitor the camera info update.
RegisterCameraInfoObserver(
pending_remote<CameraInfoObserver> observer) => ();
};

// Interface for camera device to send camera metadata to Chrome Camera App.
Expand All @@ -164,6 +164,13 @@ interface DocumentCornersObserver {
OnDocumentCornersUpdated(array<gfx.mojom.PointF> corners);
};

// Interface for monitoring the change of camera info.
interface CameraInfoObserver {
// Triggered when the camera info is updated, generally happened when the
// underlying camera is restarted.
OnCameraInfoUpdated(CameraInfo camera_info);
};

// Interface for the listener of reprocess results.
interface ReprocessResultListener {
// Triggered when reprocess done for target |effect|.
Expand Down

0 comments on commit c023c61

Please sign in to comment.