[libcamera-devel] [PATCH 3/3] android: camera_device: Use controls::SensorTimestamp

[Hirokazu Honda]

Hi Jacopo, thanks for the patch.

On Thu, Apr 8, 2021 at 1:06 AM Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Use the controls::SensorTimestamp value to populate
> ANDROID_SENSOR_TIMESTAMP result metadata, if the Camera provides
> it.
>
> Use the same control to notify the shutter even to the camera framework
> otherwise fall-back to the timestamp of the first completed buffer
> if it is not available.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 33 ++++++++++++++++++++-------------
>  src/android/camera_device.h   |  3 +--
>  2 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 89044efa7ebe..749fe5c3dedc 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -2019,7 +2019,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>
>  void CameraDevice::requestComplete(Request *request)
>  {
> -       const Request::BufferMap &buffers = request->buffers();
>         camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK;
>         std::unique_ptr<CameraMetadata> resultMetadata;
>         Camera3RequestDescriptor *descriptor =
> @@ -2034,14 +2033,7 @@ void CameraDevice::requestComplete(Request *request)
>         LOG(HAL, Debug) << "Request " << request->cookie() << " completed with "
>                         << descriptor->buffers_.size() << " streams";
>
> -       /*
> -        * \todo The timestamp used for the metadata is currently always taken
> -        * from the first buffer (which may be the first stream) in the Request.
> -        * It might be appropriate to return a 'correct' (as determined by
> -        * pipeline handlers) timestamp in the Request itself.
> -        */
> -       uint64_t timestamp = buffers.begin()->second->metadata().timestamp;
> -       resultMetadata = getResultMetadata(*descriptor, timestamp);
> +       resultMetadata = getResultMetadata(*descriptor);
>
>         /* Handle any JPEG compression. */
>         for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {
> @@ -2086,6 +2078,19 @@ void CameraDevice::requestComplete(Request *request)
>         captureResult.output_buffers = descriptor->buffers_.data();
>
>         if (status == CAMERA3_BUFFER_STATUS_OK) {
> +               int64_t timestamp;
> +
> +               if (request->metadata().contains(controls::SensorTimestamp)) {
> +                       timestamp = request->metadata().get(controls::SensorTimestamp);
> +               } else {
> +                       /*
> +                        * Use the timestamp from the first buffer (which may be
> +                        * the first stream) in the Request if the pipeline does
> +                        * not report the sensor timestamp.
> +                        */

I would leave a TODO comment of removing this after every pipeline
fills a timestamp to the metadata.

> +                       const Request::BufferMap &buffers = request->buffers();
> +                       timestamp = buffers.begin()->second->metadata().timestamp;
> +               }
>                 notifyShutter(descriptor->frameNumber_, timestamp);
>
>                 captureResult.partial_result = 1;
> @@ -2147,8 +2152,7 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)
>   * Produce a set of fixed result metadata.
>   */
>  std::unique_ptr<CameraMetadata>
> -CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor,
> -                               int64_t timestamp) const
> +CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) const
>  {
>         const ControlList &metadata = descriptor.request_->metadata();
>         const CameraMetadata &settings = descriptor.settings_;
> @@ -2274,8 +2278,6 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor,
>         resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,
>                                  &value32, 1);
>
> -       resultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, &timestamp, 1);
> -
>         value = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
>         resultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,
>                                  &value, 1);
> @@ -2314,6 +2316,11 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor,
>                                          &exposure, 1);
>         }
>
> +       if (metadata.contains(controls::SensorTimestamp)) {
> +               int64_t timestamp = metadata.get(controls::SensorTimestamp);
> +               resultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, &timestamp, 1);
> +       }
> +
>         if (metadata.contains(controls::ScalerCrop)) {
>                 Rectangle crop = metadata.get(controls::ScalerCrop);
>                 int32_t cropRect[] = {
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 11bdfec8d587..73e5009ac274 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -102,8 +102,7 @@ private:
>         libcamera::PixelFormat toPixelFormat(int format) const;
>         int processControls(Camera3RequestDescriptor *descriptor);
>         std::unique_ptr<CameraMetadata> getResultMetadata(
> -               const Camera3RequestDescriptor &descriptor,
> -               int64_t timestamp) const;
> +               const Camera3RequestDescriptor &descriptor) const;
>
>         unsigned int id_;
>         camera3_device_t camera3Device_;
> --

Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> 2.31.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel