Message ID | 20210123051704.188117-8-paul.elder@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Paul Elder |
Headers | show |
Series |
|
Related | show |
Hi Paul, Thank you for the patch. On Sat, Jan 23, 2021 at 02:17:03PM +0900, Paul Elder wrote: > The settings in an android capture request may be null, in which case > the settings from the most recently submitted capture request should be > used. Cache the request settings to achieve this. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v3: > - rebase on "android: camera device and metatada improvements", so it's > a bit cleaner > > New in v2 > --- > src/android/camera_device.cpp | 9 ++++++--- > src/android/camera_device.h | 2 ++ > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index d0922db7..a2484fdd 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -343,7 +343,8 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor() > > CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera) > : id_(id), running_(false), camera_(camera), staticMetadata_(nullptr), > - facing_(CAMERA_FACING_FRONT), orientation_(0) > + facing_(CAMERA_FACING_FRONT), orientation_(0), > + lastSettings_(CameraMetadata(0, 0)) CameraMetadata has a default constructor, wouldn't it be enough ? You could then drop this hunk completely. > { > camera_->requestCompleted.connect(this, &CameraDevice::requestComplete); > > @@ -1688,10 +1689,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > * The descriptor and the associated memory reserved here are freed > * at request complete time. > */ > - /* \todo Handle null request settings */ > - CameraMetadata settings(camera3Request->settings); > Camera3RequestDescriptor *descriptor = > new Camera3RequestDescriptor(camera_.get(), camera3Request); > + if (camera3Request->settings) > + lastSettings_ = camera3Request->settings; > + else > + descriptor->settings_ = lastSettings_; We will likely need something a bit more elaborate in the future. For instance, if a request contains ANDROID_JPEG_THUMBNAIL_SIZE and the next one doesn't, we should use the previous thumbnail size. The cache should thus be populated incrementaly. This is good enough for now, but maybe we should record a \todo ? It should mention checking whether incremental handling of the cache is required, maybe the Android camera service does so already and guarantees either null or fully populated request settings. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > LOG(HAL, Debug) << "Queueing Request to libcamera with " > << descriptor->numBuffers_ << " HAL streams"; > diff --git a/src/android/camera_device.h b/src/android/camera_device.h > index 058a3f9a..fa4fb544 100644 > --- a/src/android/camera_device.h > +++ b/src/android/camera_device.h > @@ -134,6 +134,8 @@ private: > int orientation_; > > unsigned int maxJpegBufferSize_; > + > + CameraMetadata lastSettings_; > }; > > #endif /* __ANDROID_CAMERA_DEVICE_H__ */
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index d0922db7..a2484fdd 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -343,7 +343,8 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor() CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera) : id_(id), running_(false), camera_(camera), staticMetadata_(nullptr), - facing_(CAMERA_FACING_FRONT), orientation_(0) + facing_(CAMERA_FACING_FRONT), orientation_(0), + lastSettings_(CameraMetadata(0, 0)) { camera_->requestCompleted.connect(this, &CameraDevice::requestComplete); @@ -1688,10 +1689,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques * The descriptor and the associated memory reserved here are freed * at request complete time. */ - /* \todo Handle null request settings */ - CameraMetadata settings(camera3Request->settings); Camera3RequestDescriptor *descriptor = new Camera3RequestDescriptor(camera_.get(), camera3Request); + if (camera3Request->settings) + lastSettings_ = camera3Request->settings; + else + descriptor->settings_ = lastSettings_; LOG(HAL, Debug) << "Queueing Request to libcamera with " << descriptor->numBuffers_ << " HAL streams"; diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 058a3f9a..fa4fb544 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -134,6 +134,8 @@ private: int orientation_; unsigned int maxJpegBufferSize_; + + CameraMetadata lastSettings_; }; #endif /* __ANDROID_CAMERA_DEVICE_H__ */
The settings in an android capture request may be null, in which case the settings from the most recently submitted capture request should be used. Cache the request settings to achieve this. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- Changes in v3: - rebase on "android: camera device and metatada improvements", so it's a bit cleaner New in v2 --- src/android/camera_device.cpp | 9 ++++++--- src/android/camera_device.h | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-)