[libcamera-devel] [PATCH v4 2/9] libcamera: pipeline: ipa: raspberrypi: Rework drop frame signalling
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Jul 21 12:59:53 CEST 2020
Hi Naush,
On 20/07/2020 10:13, Naushir Patuck wrote:
> The IPA now signals up front how many frames it wants the pipeline
> handler to drop. This makes it easier to handle up-coming changes to the
> buffer handling for import/export buffers.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Looks good to me
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> include/libcamera/ipa/raspberrypi.h | 2 +-
> src/ipa/raspberrypi/raspberrypi.cpp | 20 +++++------
> .../pipeline/raspberrypi/raspberrypi.cpp | 36 +++++++++++--------
> 3 files changed, 33 insertions(+), 25 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index a4937769..0f31b6ea 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -14,6 +14,7 @@ enum RPiConfigParameters {
> RPI_IPA_CONFIG_LS_TABLE = (1 << 0),
> RPI_IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
> RPI_IPA_CONFIG_SENSOR = (1 << 2),
> + RPI_IPA_CONFIG_DROP_FRAMES = (1 << 3),
> };
>
> enum RPiOperations {
> @@ -21,7 +22,6 @@ enum RPiOperations {
> RPI_IPA_ACTION_V4L2_SET_ISP,
> RPI_IPA_ACTION_STATS_METADATA_COMPLETE,
> RPI_IPA_ACTION_RUN_ISP,
> - RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,
> RPI_IPA_ACTION_EMBEDDED_COMPLETE,
> RPI_IPA_EVENT_SIGNAL_STAT_READY,
> RPI_IPA_EVENT_SIGNAL_ISP_PREPARE,
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 7bd04880..2d9fb9d8 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -65,8 +65,8 @@ class IPARPi : public IPAInterface
> public:
> IPARPi()
> : lastMode_({}), controller_(), controllerInit_(false),
> - frame_count_(0), check_count_(0), hide_count_(0),
> - mistrust_count_(0), lsTable_(nullptr)
> + frame_count_(0), check_count_(0), mistrust_count_(0),
> + lsTableHandle_(0), lsTable_(nullptr)
> {
> }
>
> @@ -137,8 +137,6 @@ private:
> uint64_t frame_count_;
> /* For checking the sequencing of Prepare/Process calls. */
> uint64_t check_count_;
> - /* How many frames the pipeline handler should hide, or "drop". */
> - unsigned int hide_count_;
> /* How many frames we should avoid running control algos on. */
> unsigned int mistrust_count_;
> /* LS table allocation passed in from the pipeline handler. */
> @@ -242,14 +240,18 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> */
> frame_count_ = 0;
> check_count_ = 0;
> + int drop_frame = 0;
> if (controllerInit_) {
> - hide_count_ = helper_->HideFramesModeSwitch();
> + drop_frame = helper_->HideFramesModeSwitch();
> mistrust_count_ = helper_->MistrustFramesModeSwitch();
> } else {
> - hide_count_ = helper_->HideFramesStartup();
> + drop_frame = helper_->HideFramesStartup();
> mistrust_count_ = helper_->MistrustFramesStartup();
> }
>
> + result->data.push_back(drop_frame);
> + result->operation |= RPI_IPA_CONFIG_DROP_FRAMES;
> +
> struct AgcStatus agcStatus;
> /* These zero values mean not program anything (unless overwritten). */
> agcStatus.shutter_time = 0.0;
> @@ -366,13 +368,11 @@ void IPARPi::processEvent(const IPAOperationData &event)
> * they are "unreliable".
> */
> prepareISP(embeddedbufferId);
> + frame_count_++;
>
> /* Ready to push the input buffer into the ISP. */
> IPAOperationData op;
> - if (++frame_count_ > hide_count_)
> - op.operation = RPI_IPA_ACTION_RUN_ISP;
> - else
> - op.operation = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME;
> + op.operation = RPI_IPA_ACTION_RUN_ISP;
The bit field flag here looks better to me indeed!
> op.data = { bayerbufferId & RPiIpaMask::ID };
> queueFrameAction.emit(0, op);
> break;
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 6630ef57..d3639ce1 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -133,7 +133,7 @@ class RPiCameraData : public CameraData
> public:
> RPiCameraData(PipelineHandler *pipe)
> : CameraData(pipe), sensor_(nullptr), state_(State::Stopped),
> - dropFrame_(false), ispOutputCount_(0)
> + dropFrameCount_(0), ispOutputCount_(0)
> {
> }
>
> @@ -181,14 +181,15 @@ public:
> std::queue<FrameBuffer *> embeddedQueue_;
> std::deque<Request *> requestQueue_;
>
> + unsigned int dropFrameCount_;
> +
> private:
> void checkRequestCompleted();
> void tryRunPipeline();
> void tryFlushQueues();
> FrameBuffer *updateQueue(std::queue<FrameBuffer *> &q, uint64_t timestamp, V4L2VideoDevice *dev);
>
> - bool dropFrame_;
> - int ispOutputCount_;
> + unsigned int ispOutputCount_;
> };
>
> class RPiCameraConfiguration : public CameraConfiguration
> @@ -999,6 +1000,7 @@ int RPiCameraData::configureIPA()
> ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,
> &result);
>
> + unsigned int resultIdx = 0;
> if (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) {
> /*
> * Setup our staggered control writer with the sensor default
> @@ -1006,9 +1008,9 @@ int RPiCameraData::configureIPA()
> */
> if (!staggeredCtrl_) {
> staggeredCtrl_.init(unicam_[Unicam::Image].dev(),
> - { { V4L2_CID_ANALOGUE_GAIN, result.data[0] },
> - { V4L2_CID_EXPOSURE, result.data[1] } });
> - sensorMetadata_ = result.data[2];
> + { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },
> + { V4L2_CID_EXPOSURE, result.data[resultIdx++] } });
> + sensorMetadata_ = result.data[resultIdx++];
> }
>
> /* Configure the H/V flip controls based on the sensor rotation. */
> @@ -1025,6 +1027,11 @@ int RPiCameraData::configureIPA()
> LOG(RPI, Error) << "V4L2 staggered set failed";
> }
>
> + if (result.operation & RPI_IPA_CONFIG_DROP_FRAMES) {
> + /* Configure the number of dropped frames required on startup. */
> + dropFrameCount_ = result.data[resultIdx++];
I assume given this is about dropping frames on startup, it only occurs
once, and dropFrameCount_ is already zero at this point, so we're not
losing a previous request to drop frames for instance.
> + }
> +
> return 0;
> }
>
> @@ -1075,7 +1082,6 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData
> break;
> }
>
> - case RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME:
> case RPI_IPA_ACTION_RUN_ISP: {
> unsigned int bufferId = action.data[0];
> FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers()->at(bufferId).get();
> @@ -1084,7 +1090,6 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData
> << ", timestamp: " << buffer->metadata().timestamp;
>
> isp_[Isp::Input].dev()->queueBuffer(buffer);
> - dropFrame_ = (action.operation == RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME) ? true : false;
> ispOutputCount_ = 0;
> break;
> }
> @@ -1250,7 +1255,7 @@ void RPiCameraData::clearIncompleteRequests()
> void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream)
> {
> if (stream->isExternal()) {
> - if (!dropFrame_) {
> + if (!dropFrameCount_) {
> Request *request = buffer->request();
> pipe_->completeBuffer(camera_, request, buffer);
> }
> @@ -1262,7 +1267,7 @@ void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream
> * simply memcpy to the Request buffer and requeue back to the
> * device.
> */
> - if (stream == &unicam_[Unicam::Image] && !dropFrame_) {
> + if (stream == &unicam_[Unicam::Image] && !dropFrameCount_) {
> const Stream *rawStream = static_cast<const Stream *>(&isp_[Isp::Input]);
> Request *request = requestQueue_.front();
> FrameBuffer *raw = request->findBuffer(const_cast<Stream *>(rawStream));
> @@ -1307,7 +1312,7 @@ void RPiCameraData::checkRequestCompleted()
> * If we are dropping this frame, do not touch the request, simply
> * change the state to IDLE when ready.
> */
> - if (!dropFrame_) {
> + if (!dropFrameCount_) {
> Request *request = requestQueue_.front();
> if (request->hasPendingBuffers())
> return;
> @@ -1326,10 +1331,13 @@ void RPiCameraData::checkRequestCompleted()
> * frame.
> */
> if (state_ == State::IpaComplete &&
> - ((ispOutputCount_ == 3 && dropFrame_) || requestCompleted)) {
> + ((ispOutputCount_ == 3 && dropFrameCount_) || requestCompleted)) {
> state_ = State::Idle;
> - if (dropFrame_)
> - LOG(RPI, Info) << "Dropping frame at the request of the IPA";
> + if (dropFrameCount_) {
> + dropFrameCount_--;
> + LOG(RPI, Info) << "Dropping frame at the request of the IPA ("
> + << dropFrameCount_ << " left)";
> + }
> }
> }
>
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list