[PATCH v6 07/18] libcamera: software_isp: Track and pass frame ids
Milan Zamazal
mzamazal at redhat.com
Thu Sep 19 18:56:49 CEST 2024
Hi Dan,
thank you for review.
Dan Scally <dan.scally at ideasonboard.com> writes:
> 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?
It doesn't seem to be needed at all, I'll remove it.
> 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