[libcamera-devel] [PATCH 2/9] libcamera: pipeline: ipa: raspberrypi: Rework drop frame signalling
Niklas Söderlund
niklas.soderlund at ragnatech.se
Mon Jul 13 13:30:37 CEST 2020
Hi Naushir,
Thanks for your work.
On 2020-07-13 09:47:21 +0100, 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.
I like this patch. I think it makes the IPA interface easier :-) As you
say however it depends on pending work and may need to be refreshed to
fit that.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> include/libcamera/ipa/raspberrypi.h | 2 +-
> src/ipa/raspberrypi/raspberrypi.cpp | 21 ++++++------
> .../pipeline/raspberrypi/raspberrypi.cpp | 33 +++++++++++--------
> 3 files changed, 32 insertions(+), 24 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index a18ce9a8..2dab2b00 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -15,8 +15,8 @@ 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_SET_SENSOR_CONFIG,
> + RPI_IPA_ACTION_SET_DROP_FRAME_COUNT,
> 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 b1f27861..f3568882 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -64,8 +64,8 @@ class IPARPi : public IPAInterface
> public:
> IPARPi()
> : lastMode_({}), controller_(), controllerInit_(false),
> - frame_count_(0), check_count_(0), hide_count_(0),
> - mistrust_count_(0), lsTableHandle_(0), lsTable_(nullptr)
> + frame_count_(0), check_count_(0), mistrust_count_(0),
> + lsTableHandle_(0), lsTable_(nullptr)
> {
> }
>
> @@ -132,8 +132,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. */
> @@ -241,14 +239,19 @@ 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();
> }
>
> + IPAOperationData op;
> + op.operation = RPI_IPA_ACTION_SET_DROP_FRAME_COUNT;
> + op.data.push_back(drop_frame);
> +
> struct AgcStatus agcStatus;
> /* These zero values mean not program anything (unless overwritten). */
> agcStatus.shutter_time = 0.0;
> @@ -339,13 +342,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;
> 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 7bae72f9..8b6f578f 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -131,7 +131,7 @@ class RPiCameraData : public CameraData
> public:
> RPiCameraData(PipelineHandler *pipe)
> : CameraData(pipe), sensor_(nullptr), lsTable_(nullptr),
> - state_(State::Stopped), dropFrame_(false), ispOutputCount_(0)
> + state_(State::Stopped), dropFrameCount_(0), ispOutputCount_(0)
> {
> }
>
> @@ -168,8 +168,8 @@ public:
>
> CameraSensor *sensor_;
> /* Array of Unicam and ISP device streams and associated buffers/streams. */
> - RPiDevice<Unicam, 2> unicam_;
> - RPiDevice<Isp, 4> isp_;
> + RPi::RPiDevice<Unicam, 2> unicam_;
> + RPi::RPiDevice<Isp, 4> isp_;
> /* The vector below is just for convenience when iterating over all streams. */
> std::vector<RPi::RPiStream *> streams_;
> /* Buffers passed to the IPA. */
> @@ -194,14 +194,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
> @@ -1058,6 +1059,11 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData
> goto done;
> }
>
> + case RPI_IPA_ACTION_SET_DROP_FRAME_COUNT: {
> + dropFrameCount_ = action.data[0];
> + goto done;
> + }
> +
> case RPI_IPA_ACTION_V4L2_SET_ISP: {
> ControlList controls = action.controls[0];
> isp_[Isp::Input].dev()->setControls(&controls);
> @@ -1091,7 +1097,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();
> @@ -1100,7 +1105,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;
> }
> @@ -1266,7 +1270,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);
> }
> @@ -1278,7 +1282,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));
> @@ -1323,7 +1327,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;
> @@ -1342,10 +1346,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)";
> + }
> }
> }
>
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list