diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 1f4fa6974bde..90c7046c66f7 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -239,7 +239,13 @@ bool DelayedControls::push(uint32_t sequence, const ControlList &controls) */ ControlList DelayedControls::get(uint32_t sequence) { - unsigned int index = std::max(0, sequence - maxDelay_); + unsigned int index = sequence; + + if (sequence > writeCount_) + LOG(DelayedControls, Error) + << "Get controls for frame " << sequence + << " but only up to frame " << writeCount_ + << " were sent to the device"; ControlList out(device_->controls()); for (const auto &ctrl : values_) { @@ -265,13 +271,14 @@ ControlList DelayedControls::get(uint32_t sequence) */ /** - * \brief Inform DelayedControls of the start of a new frame - * \param[in] sequence Sequence number of the frame that started + * \brief Apply controls for a frame + * \param[in] sequence Sequence number of the frame to apply * - * Inform the state machine that a new frame has started and of its sequence - * number. Any user of these helpers is responsible to inform the helper about - * the start of any frame. This can be connected with ease to the start of a - * exposure (SOE) V4L2 event. + * Apply controls for the frame \a sequence. This applies the controls with the + * largest delay. For controls with a smaller delay it does a look back and + * applies the controls for the previous sequence. So usually this function is + * called in a start of exposure event as applyControls(startedSequence + + * maxDelay) */ void DelayedControls::applyControls(uint32_t sequence) { diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 14cab9e5559b..1341119f6f66 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1384,7 +1384,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer) */ void IPU3CameraData::frameStart(uint32_t sequence) { - delayedCtrls_->applyControls(sequence); + delayedCtrls_->applyControls(sequence + delayedCtrls_->maxDelay()); if (processingRequests_.empty()) return; diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index 73a03373c833..70c751fc7d90 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -1849,8 +1849,10 @@ bool PipelineHandlerMaliC55::registerSensorCamera(MediaLink *ispLink) V4L2Subdevice *sensorSubdev = in->sensor_->device(); data->delayedCtrls_ = std::make_unique(sensorSubdev, params); - isp_->frameStart.connect(data->delayedCtrls_.get(), - &DelayedControls::applyControls); + isp_->frameStart.connect(data->delayedCtrls_.get(), [&](uint32_t seq) { + uint32_t lookahead = data->delayedCtrls_->maxDelay(); + data->delayedCtrls_->applyControls(seq + lookahead); + }); /* \todo Init properties. */ diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index a21b921f228c..ffd49157ec67 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1512,7 +1512,8 @@ void PipelineHandlerRkISP1::frameStart(uint32_t sequence) return; RkISP1CameraData *data = cameraData(activeCamera_); - data->delayedCtrls_->applyControls(sequence); + uint32_t sequenceToApply = sequence + data->delayedCtrls_->maxDelay(); + data->delayedCtrls_->applyControls(sequenceToApply); } bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 35c29ceca1bb..572a73307fdb 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -361,6 +361,8 @@ public: std::unique_ptr swIsp_; SimpleFrames frameInfo_; + void frameStart(uint32_t sequence); + private: void tryPipeline(unsigned int code, const Size &size); static std::vector routedSourcePads(MediaPad *sink); @@ -1058,6 +1060,11 @@ void SimpleCameraData::setSensorControls(const ControlList &sensorControls) } } +void SimpleCameraData::frameStart(uint32_t sequence) +{ + delayedCtrls_->applyControls(sequence + delayedCtrls_->maxDelay()); +} + /* Retrieve all source pads connected to a sink pad through active routes. */ std::vector SimpleCameraData::routedSourcePads(MediaPad *sink) { @@ -1671,8 +1678,8 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL stop(camera); return ret; } - frameStartEmitter->frameStart.connect(data->delayedCtrls_.get(), - &DelayedControls::applyControls); + frameStartEmitter->frameStart.connect(data, + &SimpleCameraData::frameStart); } ret = video->streamOn(); @@ -1711,8 +1718,8 @@ void SimplePipelineHandler::stopDevice(Camera *camera) if (frameStartEmitter) { frameStartEmitter->setFrameStartEnabled(false); - frameStartEmitter->frameStart.disconnect(data->delayedCtrls_.get(), - &DelayedControls::applyControls); + frameStartEmitter->frameStart.disconnect(data, + &SimpleCameraData::frameStart); } if (data->useConversion_) { diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp index 7bd30e7aead8..5bfef285bf2b 100644 --- a/test/delayed_controls.cpp +++ b/test/delayed_controls.cpp @@ -84,23 +84,20 @@ protected: dev_->setControls(&ctrls); delayed->reset(); - /* Trigger the first frame start event */ - delayed->applyControls(0); - /* Test control without delay are set at once. */ for (unsigned int i = 1; i < 100; i++) { int32_t value = 100 + i; ctrls.set(V4L2_CID_BRIGHTNESS, value); - delayed->push(ctrls); + delayed->push(i, ctrls); delayed->applyControls(i); - ControlList result = delayed->get(i); + ControlList result = delayed->get(i - delayed->maxDelay()); int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); if (brightness != value) { cerr << "Failed single control without delay" - << " frame " << i + << " frame " << i - delayed->maxDelay() << " expected " << value << " got " << brightness << endl; @@ -126,23 +123,19 @@ protected: dev_->setControls(&ctrls); delayed->reset(); - /* Trigger the first frame start event */ - delayed->applyControls(0); - /* Test single control with delay. */ for (unsigned int i = 1; i < 100; i++) { int32_t value = 10 + i; ctrls.set(V4L2_CID_BRIGHTNESS, value); - delayed->push(ctrls); - + delayed->push(i, ctrls); delayed->applyControls(i); - ControlList result = delayed->get(i); + ControlList result = delayed->get(i - delayed->maxDelay()); int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); if (brightness != expected) { cerr << "Failed single control with delay" - << " frame " << i + << " frame " << i - delayed->maxDelay() << " expected " << expected << " got " << brightness << endl; @@ -167,32 +160,38 @@ protected: std::make_unique(dev_.get(), delays); ControlList ctrls; - /* Reset control to value that will be first two frames in test. */ + /* + * Reset control to value that will be first two frames in test. + * We expect the following values: + * Frame 0 1 2 3 4 5 ... + * Brightness 200 11 12 13 14 15 + * Contrast 201 12 13 14 15 16 + */ int32_t expected = 200; ctrls.set(V4L2_CID_BRIGHTNESS, expected); ctrls.set(V4L2_CID_CONTRAST, expected + 1); dev_->setControls(&ctrls); delayed->reset(); - /* Trigger the first frame start event */ - delayed->applyControls(0); - /* Test dual control with delay. */ for (unsigned int i = 1; i < 100; i++) { int32_t value = 10 + i; ctrls.set(V4L2_CID_BRIGHTNESS, value); ctrls.set(V4L2_CID_CONTRAST, value + 1); - delayed->push(ctrls); + delayed->push(i, ctrls); delayed->applyControls(i); - ControlList result = delayed->get(i); + if (i < maxDelay) + continue; + + ControlList result = delayed->get(i - delayed->maxDelay()); int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); int32_t contrast = result.get(V4L2_CID_CONTRAST).get(); if (brightness != expected || contrast != expected + 1) { cerr << "Failed dual controls" - << " frame " << i + << " frame " << i - delayed->maxDelay() << " brightness " << brightness << " contrast " << contrast << " expected " << expected @@ -225,35 +224,35 @@ protected: dev_->setControls(&ctrls); delayed->reset(); - /* Trigger the first frame start event */ - delayed->applyControls(0); - /* * Queue all controls before any fake frame start. Note we * can't queue up more then the delayed controls history size * which is 16. Where one spot is used by the reset control. */ - for (unsigned int i = 0; i < 15; i++) { + for (unsigned int i = 1; i < 15; i++) { int32_t value = 10 + i; ctrls.set(V4L2_CID_BRIGHTNESS, value); ctrls.set(V4L2_CID_CONTRAST, value); - delayed->push(ctrls); + delayed->push(i, ctrls); } /* Process all queued controls. */ - for (unsigned int i = 1; i < 16; i++) { - int32_t value = 10 + i - 1; + for (unsigned int i = 1; i < 15; i++) { + int32_t value = 10 + i; delayed->applyControls(i); - ControlList result = delayed->get(i); + if (i < maxDelay) + continue; + + ControlList result = delayed->get(i - maxDelay); int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); int32_t contrast = result.get(V4L2_CID_CONTRAST).get(); if (brightness != expected || contrast != expected) { cerr << "Failed multi queue" - << " frame " << i + << " frame " << i - maxDelay << " brightness " << brightness << " contrast " << contrast << " expected " << expected