Message ID | 20210125071444.26252-8-paul.elder@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Paul Elder |
Headers | show |
Series |
|
Related | show |
Hi Paul, Thank you for the patch. On Mon, Jan 25, 2021 at 04:14:43PM +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> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > Changes in v4: > - use default CameraMetadata constructor for lastSettings_ (so no > initialization in CameraDevice constructor) > - add todo for incremental caching of android request settings > > 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 | 6 +++++- > src/android/camera_device.h | 2 ++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index 3068f89f..9330db39 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -1688,9 +1688,13 @@ 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 */ > Camera3RequestDescriptor *descriptor = > new Camera3RequestDescriptor(camera_.get(), camera3Request); > + /* \todo Set cache incrementally? */ I'm not sure we'll remember what this means in a few months :-) /* * \todo The Android request model is incremental, settings passed in * previous requests are to be effective until overridden explicitly in * a new request. Do we need to cache settings incrementally here, or is * it handled by the Android camera service ? */ > + 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__ */
Hi Paul On Mon, Jan 25, 2021 at 04:14:43PM +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> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> This scares me a bit, as it's not totally clear to me what the semantic of a request without control impacts the other metadata contructed using the values passed to us as settings. For the time being though, I think it's ok Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > --- > Changes in v4: > - use default CameraMetadata constructor for lastSettings_ (so no > initialization in CameraDevice constructor) > - add todo for incremental caching of android request settings > > 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 | 6 +++++- > src/android/camera_device.h | 2 ++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index 3068f89f..9330db39 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -1688,9 +1688,13 @@ 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 */ > Camera3RequestDescriptor *descriptor = > new Camera3RequestDescriptor(camera_.get(), camera3Request); > + /* \todo Set cache incrementally? */ > + 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__ */ > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 3068f89f..9330db39 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1688,9 +1688,13 @@ 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 */ Camera3RequestDescriptor *descriptor = new Camera3RequestDescriptor(camera_.get(), camera3Request); + /* \todo Set cache incrementally? */ + 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__ */