[PATCH v6 07/18] libcamera: software_isp: Track and pass frame ids
Dan Scally
dan.scally at ideasonboard.com
Mon Sep 9 10:10:39 CEST 2024
Hi Milan - thanks for the patch
On 06/09/2024 13:09, Milan Zamazal wrote:
> A previous preparation patch implemented passing frame ids to stats
> processing but without actual meaningful frame id value passed there.
> This patch extends that by actually providing the frame id and passing
> it through to the stats processor.
>
> The frame id is taken from the request sequence number, the same as in
> hardware pipelines.
>
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
> .../libcamera/internal/software_isp/software_isp.h | 4 ++--
> src/libcamera/pipeline/simple/simple.cpp | 8 +++++++-
> src/libcamera/software_isp/debayer.cpp | 3 ++-
> src/libcamera/software_isp/debayer.h | 2 +-
> src/libcamera/software_isp/debayer_cpu.cpp | 9 ++++-----
> src/libcamera/software_isp/debayer_cpu.h | 2 +-
> src/libcamera/software_isp/software_isp.cpp | 11 +++++++----
> 7 files changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index 3602bce8..3a84418e 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -73,10 +73,10 @@ public:
> int start();
> void stop();
>
> - int queueBuffers(FrameBuffer *input,
> + int queueBuffers(uint32_t frame, FrameBuffer *input,
> const std::map<const Stream *, FrameBuffer *> &outputs);
>
> - void process(FrameBuffer *input, FrameBuffer *output);
> + void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output);
>
> Signal<FrameBuffer *> inputBufferReady;
> Signal<FrameBuffer *> outputBufferReady;
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 48a568da..ebec592a 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -865,7 +865,13 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
> if (converter_)
> converter_->queueBuffers(buffer, conversionQueue_.front());
> else
> - swIsp_->queueBuffers(buffer, conversionQueue_.front());
> + /*
> + * request->sequence() cannot be retrieved from `buffer' inside
> + * queueBuffers because unique_ptr's make buffer->request() invalid
> + * already here.
> + */
> + swIsp_->queueBuffers(request->sequence(), buffer,
> + conversionQueue_.front());
>
> conversionQueue_.pop();
> return;
> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
> index db26c380..f0b83261 100644
> --- a/src/libcamera/software_isp/debayer.cpp
> +++ b/src/libcamera/software_isp/debayer.cpp
> @@ -94,8 +94,9 @@ Debayer::~Debayer()
> */
>
> /**
> - * \fn void Debayer::process(FrameBuffer *input, FrameBuffer *output, DebayerParams params)
> + * \fn void Debayer::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params)
> * \brief Process the bayer data into the requested format
> + * \param[in] frame The frame number
> * \param[in] input The input buffer
> * \param[in] output The output buffer
> * \param[in] params The parameters to be used in debayering
> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
> index c151fe5d..d7ca060d 100644
> --- a/src/libcamera/software_isp/debayer.h
> +++ b/src/libcamera/software_isp/debayer.h
> @@ -40,7 +40,7 @@ public:
> virtual std::tuple<unsigned int, unsigned int>
> strideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0;
>
> - virtual void process(FrameBuffer *input, FrameBuffer *output, DebayerParams params) = 0;
> + virtual void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params) = 0;
>
> virtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;
>
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index 2a2e7edb..f7b3a7d1 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -724,7 +724,7 @@ static inline int64_t timeDiff(timespec &after, timespec &before)
> (int64_t)after.tv_nsec - (int64_t)before.tv_nsec;
> }
>
> -void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams params)
> +void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params)
> {
> timespec frameStartTime;
>
> @@ -778,12 +778,11 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams
> }
>
> /*
> - * Frame and buffer ids are currently not used, so pass zeros as parameters.
> + * Buffer ids are currently not used, so pass zeros as its parameter.
> *
> - * \todo Pass real values once frame is passed here and stats buffer passing
> - * is changed.
> + * \todo Pass real bufferId once stats buffer passing is changed.
> */
> - stats_->finishFrame(0, 0);
> + stats_->finishFrame(frame, 0);
> outputBufferReady.emit(output);
> inputBufferReady.emit(input);
> }
> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
> index 8237a64b..2c47e7c6 100644
> --- a/src/libcamera/software_isp/debayer_cpu.h
> +++ b/src/libcamera/software_isp/debayer_cpu.h
> @@ -36,7 +36,7 @@ public:
> std::vector<PixelFormat> formats(PixelFormat input);
> std::tuple<unsigned int, unsigned int>
> strideAndFrameSize(const PixelFormat &outputFormat, const Size &size);
> - void process(FrameBuffer *input, FrameBuffer *output, DebayerParams params);
> + void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params);
> SizeRange sizes(PixelFormat inputFormat, const Size &inputSize);
>
> /**
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index a3855568..c5db45ae 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -14,6 +14,7 @@
> #include <unistd.h>
>
> #include <libcamera/formats.h>
> +#include <libcamera/request.h>
Is this needed? I can't see that it is, at least in this patch? Otherwise;
Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>
> #include <libcamera/stream.h>
>
> #include "libcamera/internal/ipa_manager.h"
> @@ -278,12 +279,13 @@ int SoftwareIsp::exportBuffers(const Stream *stream, unsigned int count,
>
> /**
> * \brief Queue buffers to Software ISP
> + * \param[in] frame The frame number
> * \param[in] input The input framebuffer
> * \param[in] outputs The container holding the output stream pointers and
> * their respective frame buffer outputs
> * \return 0 on success, a negative errno on failure
> */
> -int SoftwareIsp::queueBuffers(FrameBuffer *input,
> +int SoftwareIsp::queueBuffers(uint32_t frame, FrameBuffer *input,
> const std::map<const Stream *, FrameBuffer *> &outputs)
> {
> /*
> @@ -301,7 +303,7 @@ int SoftwareIsp::queueBuffers(FrameBuffer *input,
> }
>
> for (auto iter = outputs.begin(); iter != outputs.end(); iter++)
> - process(input, iter->second);
> + process(frame, input, iter->second);
>
> return 0;
> }
> @@ -333,13 +335,14 @@ void SoftwareIsp::stop()
>
> /**
> * \brief Passes the input framebuffer to the ISP worker to process
> + * \param[in] frame The frame number
> * \param[in] input The input framebuffer
> * \param[out] output The framebuffer to write the processed frame to
> */
> -void SoftwareIsp::process(FrameBuffer *input, FrameBuffer *output)
> +void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output)
> {
> debayer_->invokeMethod(&DebayerCpu::process,
> - ConnectionTypeQueued, input, output, debayerParams_);
> + ConnectionTypeQueued, frame, input, output, debayerParams_);
> }
>
> void SoftwareIsp::saveIspParams()
More information about the libcamera-devel
mailing list