Skip to content

Commit

Permalink
libcamera: software_isp: Make stats frame and buffer aware
Browse files Browse the repository at this point in the history
This patch adds frame and bufferId arguments to stats related calls.
Although the parameters are currently unused, because frame ids are not
tracked and used and the stats buffer is passed around directly rather
than being referred by its id, they bring the internal APIs closer to
their counterparts in hardware pipelines.

It serves as a preparation for followup patches that will introduce:

- Frame number tracking in order to switch to DelayedControls
  (software ISP TODO #11 + #12).
- A ring buffer for stats in order to improve passing the stats
  (software ISP TODO #2).

Frame and buffer ids are unrelated for the given purposes but since they
are passed together at the same places, the change is implemented as a
single patch rather than two, basically the same, patches.

Signed-off-by: Milan Zamazal <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Daniel Scally <[email protected]>
Signed-off-by: Kieran Bingham <[email protected]>
  • Loading branch information
mz-pdm authored and kbingham committed Sep 27, 2024
1 parent 0cc74c4 commit c64f2f1
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 18 deletions.
8 changes: 5 additions & 3 deletions include/libcamera/internal/software_isp/software_isp.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <initializer_list>
#include <map>
#include <memory>
#include <stdint.h>
#include <string>
#include <tuple>
#include <vector>
Expand Down Expand Up @@ -66,7 +67,8 @@ class SoftwareIsp
int exportBuffers(const Stream *stream, unsigned int count,
std::vector<std::unique_ptr<FrameBuffer>> *buffers);

void processStats(const ControlList &sensorControls);
void processStats(const uint32_t frame, const uint32_t bufferId,
const ControlList &sensorControls);

int start();
void stop();
Expand All @@ -78,13 +80,13 @@ class SoftwareIsp

Signal<FrameBuffer *> inputBufferReady;
Signal<FrameBuffer *> outputBufferReady;
Signal<> ispStatsReady;
Signal<uint32_t, uint32_t> ispStatsReady;
Signal<const ControlList &> setSensorControls;

private:
void saveIspParams();
void setSensorCtrls(const ControlList &sensorControls);
void statsReady();
void statsReady(uint32_t frame, uint32_t bufferId);
void inputReady(FrameBuffer *input);
void outputReady(FrameBuffer *output);

Expand Down
4 changes: 3 additions & 1 deletion include/libcamera/ipa/soft.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ interface IPASoftInterface {
configure(libcamera.ControlInfoMap sensorCtrlInfoMap)
=> (int32 ret);

[async] processStats(libcamera.ControlList sensorControls);
[async] processStats(uint32 frame,
uint32 bufferId,
libcamera.ControlList sensorControls);
};

interface IPASoftEventInterface {
Expand Down
7 changes: 5 additions & 2 deletions src/ipa/simple/soft_simple.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module
int start() override;
void stop() override;

void processStats(const ControlList &sensorControls) override;
void processStats(const uint32_t frame, const uint32_t bufferId,
const ControlList &sensorControls) override;

protected:
std::string logPrefix() const override;
Expand Down Expand Up @@ -249,7 +250,9 @@ void IPASoftSimple::stop()
{
}

void IPASoftSimple::processStats(const ControlList &sensorControls)
void IPASoftSimple::processStats([[maybe_unused]] const uint32_t frame,
[[maybe_unused]] const uint32_t bufferId,
const ControlList &sensorControls)
{
SwIspStats::Histogram histogram = stats_->yHistogram;
if (ignoreUpdates_ > 0)
Expand Down
8 changes: 5 additions & 3 deletions src/libcamera/pipeline/simple/simple.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <memory>
#include <queue>
#include <set>
#include <stdint.h>
#include <string.h>
#include <string>
#include <unordered_map>
Expand Down Expand Up @@ -291,7 +292,7 @@ class SimpleCameraData : public Camera::Private
void conversionInputDone(FrameBuffer *buffer);
void conversionOutputDone(FrameBuffer *buffer);

void ispStatsReady();
void ispStatsReady(uint32_t frame, uint32_t bufferId);
void setSensorControls(const ControlList &sensorControls);
};

Expand Down Expand Up @@ -887,10 +888,11 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)
pipe->completeRequest(request);
}

void SimpleCameraData::ispStatsReady()
void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)
{
/* \todo Use the DelayedControls class */
swIsp_->processStats(sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,
swIsp_->processStats(frame, bufferId,
sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,
V4L2_CID_EXPOSURE }));
}

Expand Down
8 changes: 7 additions & 1 deletion src/libcamera/software_isp/debayer_cpu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,13 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams
}
}

stats_->finishFrame();
/*
* Frame and buffer ids are currently not used, so pass zeros as parameters.
*
* \todo Pass real values once frame is passed here and stats buffer passing
* is changed.
*/
stats_->finishFrame(0, 0);
outputBufferReady.emit(output);
inputBufferReady.emit(input);
}
Expand Down
11 changes: 7 additions & 4 deletions src/libcamera/software_isp/software_isp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,15 +155,18 @@ SoftwareIsp::~SoftwareIsp()

/**
* \brief Process the statistics gathered
* \param[in] frame The frame number
* \param[in] bufferId ID of the statistics buffer
* \param[in] sensorControls The sensor controls
*
* Requests the IPA to calculate new parameters for ISP and new control
* values for the sensor.
*/
void SoftwareIsp::processStats(const ControlList &sensorControls)
void SoftwareIsp::processStats(const uint32_t frame, const uint32_t bufferId,
const ControlList &sensorControls)
{
ASSERT(ipa_);
ipa_->processStats(sensorControls);
ipa_->processStats(frame, bufferId, sensorControls);
}

/**
Expand Down Expand Up @@ -349,9 +352,9 @@ void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)
setSensorControls.emit(sensorControls);
}

void SoftwareIsp::statsReady()
void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)
{
ispStatsReady.emit();
ispStatsReady.emit(frame, bufferId);
}

void SoftwareIsp::inputReady(FrameBuffer *input)
Expand Down
6 changes: 4 additions & 2 deletions src/libcamera/software_isp/swstats_cpu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,13 +311,15 @@ void SwStatsCpu::startFrame(void)

/**
* \brief Finish statistics calculation for the current frame
* \param[in] frame The frame number
* \param[in] bufferId ID of the statistics buffer
*
* This may only be called after a successful setWindow() call.
*/
void SwStatsCpu::finishFrame(void)
void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId)
{
*sharedStats_ = stats_;
statsReady.emit();
statsReady.emit(frame, bufferId);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/libcamera/software_isp/swstats_cpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class SwStatsCpu
int configure(const StreamConfiguration &inputCfg);
void setWindow(const Rectangle &window);
void startFrame();
void finishFrame();
void finishFrame(uint32_t frame, uint32_t bufferId);

void processLine0(unsigned int y, const uint8_t *src[])
{
Expand All @@ -61,7 +61,7 @@ class SwStatsCpu
(this->*stats2_)(src);
}

Signal<> statsReady;
Signal<uint32_t, uint32_t> statsReady;

private:
using statsProcessFn = void (SwStatsCpu::*)(const uint8_t *src[]);
Expand Down

0 comments on commit c64f2f1

Please sign in to comment.