[PATCH v6 07/18] libcamera: software_isp: Track and pass frame ids
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Sep 9 17:45:59 CEST 2024
Quoting Dan Scally (2024-09-09 09:10:39)
> 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.
I think we'll change/fix this in the future to make everything more
consistent for the different 'clock' domains in regards to PFC - but I
think it's fine for now.
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
> > 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