| Message ID | 20261013102256.41602-1-robert.mader@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Ouch, sorry, forgot to remove a debug change. Send out v3, please ignore this revision. On 13.10.26 12:22, Robert Mader wrote: > Throttle requests to the minimal frame duration in order to honor the > configured framerates. Client-requested values are ignored for now. > > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > --- > > Changes in v2: > - Remove debug log in order to avoid CI issues. > - Simplify code in queueRequestDevice(), hopefully making it easier to > follow. > > Changes in v1 (from RFC version): > - Use std::this_thread::sleep_for() instead usleep() (Barnabás) > - Sleep only once per request, after generating all frames > - Remove control handling in start() (defer that for later) > - Reset state in stop() instead of start() > --- > src/libcamera/pipeline/virtual/virtual.cpp | 42 ++++++++++++++++++---- > src/libcamera/pipeline/virtual/virtual.h | 1 + > 2 files changed, 37 insertions(+), 6 deletions(-) > > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > index e25dd85a7391..5f00f7cc6b37 100644 > --- a/src/libcamera/pipeline/virtual/virtual.cpp > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > @@ -130,6 +130,9 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe, > > void VirtualCameraData::processRequest(Request *request) > { > + int64_t sensorTimestamp = *request->metadata().get(controls::SensorTimestamp); > + std::vector<FrameBuffer *> buffers; > + > for (const auto &[stream, buffer] : request->buffers()) { > bool found = false; > /* map buffer and fill test patterns */ > @@ -139,7 +142,7 @@ void VirtualCameraData::processRequest(Request *request) > > fmd.status = FrameMetadata::Status::FrameSuccess; > fmd.sequence = streamConfig.seq++; > - fmd.timestamp = currentTimestamp(); > + fmd.timestamp = sensorTimestamp; > > std::span<const FrameBuffer::Plane> planes = buffer->planes(); > for (const auto [i, p] : utils::enumerate(planes)) > @@ -151,12 +154,21 @@ void VirtualCameraData::processRequest(Request *request) > stream->configuration().size, buffer)) > fmd.status = FrameMetadata::Status::FrameError; > > - bufferCompleted.emit(buffer); > + buffers.push_back(buffer); > break; > } > } > ASSERT(found); > } > + > + const uint64_t now = currentTimestamp(); > + if (now < static_cast<uint64_t>(sensorTimestamp)) { > + std::chrono::nanoseconds duration(sensorTimestamp - now); > + std::this_thread::sleep_for(duration); > + } > + > + for (auto buffer : buffers) > + bufferCompleted.emit(buffer); > } > > VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data) > @@ -349,18 +361,36 @@ void PipelineHandlerVirtual::stopDevice(Camera *camera) > /* Process pending `bufferCompleted` signals. */ > thread()->dispatchMessages(Message::Type::InvokeMessage, this); > data->bufferCompleted.disconnect(this); > + data->lastTimestamp = 0; > > while (!data->queuedRequests_.empty()) > cancelRequest(data->queuedRequests_.front()); > } > > -int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > - Request *request) > +int PipelineHandlerVirtual::queueRequestDevice(Camera *camera, Request *request) > { > VirtualCameraData *data = cameraData(camera); > - const auto timestamp = currentTimestamp(); > + const uint64_t now = currentTimestamp(); > + uint64_t minFrameDurationNsec; > + uint64_t targetTimestamp; > + > + auto frameDurationLimits = camera->controls().find(&controls::FrameDurationLimits); > + if (false && frameDurationLimits != camera->controls().end()) { > + minFrameDurationNsec = > + frameDurationLimits->second.min().get<int64_t>() * 1000; > + } else { > + /* Default to 30 FPS */ > + minFrameDurationNsec = 33333 * 1000; > + } > + > + targetTimestamp = data->lastTimestamp + minFrameDurationNsec; > + if (targetTimestamp < now) > + targetTimestamp = now; > + data->lastTimestamp = targetTimestamp; > + > + request->_d()->metadata().set(controls::SensorTimestamp, > + targetTimestamp); > > - request->_d()->metadata().set(controls::SensorTimestamp, timestamp); > data->invokeMethod(&VirtualCameraData::processRequest, > ConnectionTypeQueued, request); > > diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h > index 215e56fa3b9d..84b53dfb66f3 100644 > --- a/src/libcamera/pipeline/virtual/virtual.h > +++ b/src/libcamera/pipeline/virtual/virtual.h > @@ -66,6 +66,7 @@ public: > > std::vector<StreamConfig> streamConfigs_; > Signal<FrameBuffer *> bufferCompleted; > + uint64_t lastTimestamp = 0; > }; > > } /* namespace libcamera */
Quoting Robert Mader (2026-10-13 11:22:56) > Throttle requests to the minimal frame duration in order to honor the > configured framerates. Client-requested values are ignored for now. > > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > --- > > Changes in v2: > - Remove debug log in order to avoid CI issues. > - Simplify code in queueRequestDevice(), hopefully making it easier to > follow. > > Changes in v1 (from RFC version): > - Use std::this_thread::sleep_for() instead usleep() (Barnabás) > - Sleep only once per request, after generating all frames > - Remove control handling in start() (defer that for later) > - Reset state in stop() instead of start() > --- > src/libcamera/pipeline/virtual/virtual.cpp | 42 ++++++++++++++++++---- > src/libcamera/pipeline/virtual/virtual.h | 1 + > 2 files changed, 37 insertions(+), 6 deletions(-) > > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > index e25dd85a7391..5f00f7cc6b37 100644 > --- a/src/libcamera/pipeline/virtual/virtual.cpp > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > @@ -130,6 +130,9 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe, > > void VirtualCameraData::processRequest(Request *request) > { > + int64_t sensorTimestamp = *request->metadata().get(controls::SensorTimestamp); > + std::vector<FrameBuffer *> buffers; > + > for (const auto &[stream, buffer] : request->buffers()) { > bool found = false; > /* map buffer and fill test patterns */ > @@ -139,7 +142,7 @@ void VirtualCameraData::processRequest(Request *request) > > fmd.status = FrameMetadata::Status::FrameSuccess; > fmd.sequence = streamConfig.seq++; > - fmd.timestamp = currentTimestamp(); > + fmd.timestamp = sensorTimestamp; > > std::span<const FrameBuffer::Plane> planes = buffer->planes(); > for (const auto [i, p] : utils::enumerate(planes)) > @@ -151,12 +154,21 @@ void VirtualCameraData::processRequest(Request *request) > stream->configuration().size, buffer)) > fmd.status = FrameMetadata::Status::FrameError; > > - bufferCompleted.emit(buffer); > + buffers.push_back(buffer); > break; > } > } > ASSERT(found); > } > + > + const uint64_t now = currentTimestamp(); > + if (now < static_cast<uint64_t>(sensorTimestamp)) { > + std::chrono::nanoseconds duration(sensorTimestamp - now); > + std::this_thread::sleep_for(duration); > + } > + > + for (auto buffer : buffers) > + bufferCompleted.emit(buffer); > } > > VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data) > @@ -349,18 +361,36 @@ void PipelineHandlerVirtual::stopDevice(Camera *camera) > /* Process pending `bufferCompleted` signals. */ > thread()->dispatchMessages(Message::Type::InvokeMessage, this); > data->bufferCompleted.disconnect(this); > + data->lastTimestamp = 0; > > while (!data->queuedRequests_.empty()) > cancelRequest(data->queuedRequests_.front()); > } > > -int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > - Request *request) > +int PipelineHandlerVirtual::queueRequestDevice(Camera *camera, Request *request) > { > VirtualCameraData *data = cameraData(camera); > - const auto timestamp = currentTimestamp(); > + const uint64_t now = currentTimestamp(); > + uint64_t minFrameDurationNsec; > + uint64_t targetTimestamp; > + > + auto frameDurationLimits = camera->controls().find(&controls::FrameDurationLimits); > + if (false && frameDurationLimits != camera->controls().end()) { > + minFrameDurationNsec = > + frameDurationLimits->second.min().get<int64_t>() * 1000; > + } else { > + /* Default to 30 FPS */ > + minFrameDurationNsec = 33333 * 1000; > + } Something here can't be right. The camera->controls() is a list of the controls the camera supports, not what the application has asked for. The request brings in what applications ask for - i.e. to set/change the FrameDurationLimits, so either the FrameDurationLimits control is always there or ... Wait - now I see you have "if (false &&" so you already know there's dead code there. Lets not add dead code ... espcially dead code which is incorrect/misleading when/if enabled. I don't think you need to add an IPA here, but if you want to be able to process FrameDurationLimits, you need to register it as a control in the info map with limits, and during queueRequest you can parse it and update a local frameDuration variable. That's then something you can use to determine the next timestamp. -- Kieran > + > + targetTimestamp = data->lastTimestamp + minFrameDurationNsec; > + if (targetTimestamp < now) > + targetTimestamp = now; > + data->lastTimestamp = targetTimestamp; > + > + request->_d()->metadata().set(controls::SensorTimestamp, > + targetTimestamp); > > - request->_d()->metadata().set(controls::SensorTimestamp, timestamp); > data->invokeMethod(&VirtualCameraData::processRequest, > ConnectionTypeQueued, request); > > diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h > index 215e56fa3b9d..84b53dfb66f3 100644 > --- a/src/libcamera/pipeline/virtual/virtual.h > +++ b/src/libcamera/pipeline/virtual/virtual.h > @@ -66,6 +66,7 @@ public: > > std::vector<StreamConfig> streamConfigs_; > Signal<FrameBuffer *> bufferCompleted; > + uint64_t lastTimestamp = 0; > }; > > } /* namespace libcamera */ > -- > 2.56.0 >
Quoting Kieran Bingham (2026-10-13 11:34:07) > Quoting Robert Mader (2026-10-13 11:22:56) > > Throttle requests to the minimal frame duration in order to honor the > > configured framerates. Client-requested values are ignored for now. > > > > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > > > --- > > > > Changes in v2: > > - Remove debug log in order to avoid CI issues. > > - Simplify code in queueRequestDevice(), hopefully making it easier to > > follow. > > > > Changes in v1 (from RFC version): > > - Use std::this_thread::sleep_for() instead usleep() (Barnabás) > > - Sleep only once per request, after generating all frames > > - Remove control handling in start() (defer that for later) > > - Reset state in stop() instead of start() > > --- > > src/libcamera/pipeline/virtual/virtual.cpp | 42 ++++++++++++++++++---- > > src/libcamera/pipeline/virtual/virtual.h | 1 + > > 2 files changed, 37 insertions(+), 6 deletions(-) > > > > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > > index e25dd85a7391..5f00f7cc6b37 100644 > > --- a/src/libcamera/pipeline/virtual/virtual.cpp > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > > @@ -130,6 +130,9 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe, > > > > void VirtualCameraData::processRequest(Request *request) > > { > > + int64_t sensorTimestamp = *request->metadata().get(controls::SensorTimestamp); > > + std::vector<FrameBuffer *> buffers; > > + > > for (const auto &[stream, buffer] : request->buffers()) { > > bool found = false; > > /* map buffer and fill test patterns */ > > @@ -139,7 +142,7 @@ void VirtualCameraData::processRequest(Request *request) > > > > fmd.status = FrameMetadata::Status::FrameSuccess; > > fmd.sequence = streamConfig.seq++; > > - fmd.timestamp = currentTimestamp(); > > + fmd.timestamp = sensorTimestamp; > > > > std::span<const FrameBuffer::Plane> planes = buffer->planes(); > > for (const auto [i, p] : utils::enumerate(planes)) > > @@ -151,12 +154,21 @@ void VirtualCameraData::processRequest(Request *request) > > stream->configuration().size, buffer)) > > fmd.status = FrameMetadata::Status::FrameError; > > > > - bufferCompleted.emit(buffer); > > + buffers.push_back(buffer); > > break; > > } > > } > > ASSERT(found); > > } > > + > > + const uint64_t now = currentTimestamp(); > > + if (now < static_cast<uint64_t>(sensorTimestamp)) { > > + std::chrono::nanoseconds duration(sensorTimestamp - now); > > + std::this_thread::sleep_for(duration); > > + } > > + > > + for (auto buffer : buffers) > > + bufferCompleted.emit(buffer); > > } > > > > VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data) > > @@ -349,18 +361,36 @@ void PipelineHandlerVirtual::stopDevice(Camera *camera) > > /* Process pending `bufferCompleted` signals. */ > > thread()->dispatchMessages(Message::Type::InvokeMessage, this); > > data->bufferCompleted.disconnect(this); > > + data->lastTimestamp = 0; > > > > while (!data->queuedRequests_.empty()) > > cancelRequest(data->queuedRequests_.front()); > > } > > > > -int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > > - Request *request) > > +int PipelineHandlerVirtual::queueRequestDevice(Camera *camera, Request *request) > > { > > VirtualCameraData *data = cameraData(camera); > > - const auto timestamp = currentTimestamp(); > > + const uint64_t now = currentTimestamp(); > > + uint64_t minFrameDurationNsec; > > + uint64_t targetTimestamp; > > + > > + auto frameDurationLimits = camera->controls().find(&controls::FrameDurationLimits); > > + if (false && frameDurationLimits != camera->controls().end()) { > > + minFrameDurationNsec = > > + frameDurationLimits->second.min().get<int64_t>() * 1000; > > + } else { > > + /* Default to 30 FPS */ > > + minFrameDurationNsec = 33333 * 1000; > > + } > > Something here can't be right. > > The camera->controls() is a list of the controls the camera supports, > not what the application has asked for. > > The request brings in what applications ask for - i.e. to set/change the > FrameDurationLimits, so either the FrameDurationLimits control is always > there or ... > > Wait - now I see you have "if (false &&" so you already know there's > dead code there. > > Lets not add dead code ... espcially dead code which is > incorrect/misleading when/if enabled. > > I don't think you need to add an IPA here, but if you want to be able to > process FrameDurationLimits, you need to register it as a control in the > info map with limits, and during queueRequest you can parse it and > update a local frameDuration variable. > > That's then something you can use to determine the next timestamp. I see you did mention that client requests aren't handled yet, so if that's the case, lets not add the code. But I think you could handle them easily. But also - either way, you should set the FrameDuration metadata. > > -- > Kieran > > > > > + > > + targetTimestamp = data->lastTimestamp + minFrameDurationNsec; > > + if (targetTimestamp < now) > > + targetTimestamp = now; > > + data->lastTimestamp = targetTimestamp; > > + > > + request->_d()->metadata().set(controls::SensorTimestamp, > > + targetTimestamp); > > > > - request->_d()->metadata().set(controls::SensorTimestamp, timestamp); > > data->invokeMethod(&VirtualCameraData::processRequest, > > ConnectionTypeQueued, request); > > > > diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h > > index 215e56fa3b9d..84b53dfb66f3 100644 > > --- a/src/libcamera/pipeline/virtual/virtual.h > > +++ b/src/libcamera/pipeline/virtual/virtual.h > > @@ -66,6 +66,7 @@ public: > > > > std::vector<StreamConfig> streamConfigs_; > > Signal<FrameBuffer *> bufferCompleted; > > + uint64_t lastTimestamp = 0; > > }; > > > > } /* namespace libcamera */ > > -- > > 2.56.0 > >
Hi On 13.10.26 12:36, Kieran Bingham wrote: > Quoting Kieran Bingham (2026-10-13 11:34:07) >> Quoting Robert Mader (2026-10-13 11:22:56) >>> Throttle requests to the minimal frame duration in order to honor the >>> configured framerates. Client-requested values are ignored for now. >>> >>> Signed-off-by: Robert Mader <robert.mader@collabora.com> >>> >>> --- >>> >>> Changes in v2: >>> - Remove debug log in order to avoid CI issues. >>> - Simplify code in queueRequestDevice(), hopefully making it easier to >>> follow. >>> >>> Changes in v1 (from RFC version): >>> - Use std::this_thread::sleep_for() instead usleep() (Barnabás) >>> - Sleep only once per request, after generating all frames >>> - Remove control handling in start() (defer that for later) >>> - Reset state in stop() instead of start() >>> --- >>> src/libcamera/pipeline/virtual/virtual.cpp | 42 ++++++++++++++++++---- >>> src/libcamera/pipeline/virtual/virtual.h | 1 + >>> 2 files changed, 37 insertions(+), 6 deletions(-) >>> >>> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp >>> index e25dd85a7391..5f00f7cc6b37 100644 >>> --- a/src/libcamera/pipeline/virtual/virtual.cpp >>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp >>> @@ -130,6 +130,9 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe, >>> >>> void VirtualCameraData::processRequest(Request *request) >>> { >>> + int64_t sensorTimestamp = *request->metadata().get(controls::SensorTimestamp); >>> + std::vector<FrameBuffer *> buffers; >>> + >>> for (const auto &[stream, buffer] : request->buffers()) { >>> bool found = false; >>> /* map buffer and fill test patterns */ >>> @@ -139,7 +142,7 @@ void VirtualCameraData::processRequest(Request *request) >>> >>> fmd.status = FrameMetadata::Status::FrameSuccess; >>> fmd.sequence = streamConfig.seq++; >>> - fmd.timestamp = currentTimestamp(); >>> + fmd.timestamp = sensorTimestamp; >>> >>> std::span<const FrameBuffer::Plane> planes = buffer->planes(); >>> for (const auto [i, p] : utils::enumerate(planes)) >>> @@ -151,12 +154,21 @@ void VirtualCameraData::processRequest(Request *request) >>> stream->configuration().size, buffer)) >>> fmd.status = FrameMetadata::Status::FrameError; >>> >>> - bufferCompleted.emit(buffer); >>> + buffers.push_back(buffer); >>> break; >>> } >>> } >>> ASSERT(found); >>> } >>> + >>> + const uint64_t now = currentTimestamp(); >>> + if (now < static_cast<uint64_t>(sensorTimestamp)) { >>> + std::chrono::nanoseconds duration(sensorTimestamp - now); >>> + std::this_thread::sleep_for(duration); >>> + } >>> + >>> + for (auto buffer : buffers) >>> + bufferCompleted.emit(buffer); >>> } >>> >>> VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data) >>> @@ -349,18 +361,36 @@ void PipelineHandlerVirtual::stopDevice(Camera *camera) >>> /* Process pending `bufferCompleted` signals. */ >>> thread()->dispatchMessages(Message::Type::InvokeMessage, this); >>> data->bufferCompleted.disconnect(this); >>> + data->lastTimestamp = 0; >>> >>> while (!data->queuedRequests_.empty()) >>> cancelRequest(data->queuedRequests_.front()); >>> } >>> >>> -int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, >>> - Request *request) >>> +int PipelineHandlerVirtual::queueRequestDevice(Camera *camera, Request *request) >>> { >>> VirtualCameraData *data = cameraData(camera); >>> - const auto timestamp = currentTimestamp(); >>> + const uint64_t now = currentTimestamp(); >>> + uint64_t minFrameDurationNsec; >>> + uint64_t targetTimestamp; >>> + >>> + auto frameDurationLimits = camera->controls().find(&controls::FrameDurationLimits); >>> + if (false && frameDurationLimits != camera->controls().end()) { >>> + minFrameDurationNsec = >>> + frameDurationLimits->second.min().get<int64_t>() * 1000; >>> + } else { >>> + /* Default to 30 FPS */ >>> + minFrameDurationNsec = 33333 * 1000; >>> + } >> Something here can't be right. >> >> The camera->controls() is a list of the controls the camera supports, >> not what the application has asked for. >> >> The request brings in what applications ask for - i.e. to set/change the >> FrameDurationLimits, so either the FrameDurationLimits control is always >> there or ... >> >> Wait - now I see you have "if (false &&" so you already know there's >> dead code there. >> >> Lets not add dead code ... espcially dead code which is >> incorrect/misleading when/if enabled. Yeah, sorry, fixed that in v3 - the left-over false was from testing the else path, however I think we can't ever hit it - the config parser should always set some value. So I should probably just put an assert here. >> >> I don't think you need to add an IPA here, but if you want to be able to >> process FrameDurationLimits, you need to register it as a control in the >> info map with limits, and during queueRequest you can parse it and >> update a local frameDuration variable. >> >> That's then something you can use to determine the next timestamp. > I see you did mention that client requests aren't handled yet, so if > that's the case, lets not add the code. But I think you could handle > them easily. I would like to do so, however I both wasn't sure how to do it (do I need to re-create the control map? Or can I just somehow set the value?) and how to test it (I guess with camshark?). Going forward I'd like to see the control handled by the Pipewire plugin as well. Either way, I'd prefer to do so as follow-up. > But also - either way, you should set the FrameDuration metadata. You mean to the request metadata, just like we already do for controls::SensorTimestamp? Sure, can do so. > >> -- >> Kieran >> >> >> >>> + >>> + targetTimestamp = data->lastTimestamp + minFrameDurationNsec; >>> + if (targetTimestamp < now) >>> + targetTimestamp = now; >>> + data->lastTimestamp = targetTimestamp; >>> + >>> + request->_d()->metadata().set(controls::SensorTimestamp, >>> + targetTimestamp); >>> >>> - request->_d()->metadata().set(controls::SensorTimestamp, timestamp); >>> data->invokeMethod(&VirtualCameraData::processRequest, >>> ConnectionTypeQueued, request); >>> >>> diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h >>> index 215e56fa3b9d..84b53dfb66f3 100644 >>> --- a/src/libcamera/pipeline/virtual/virtual.h >>> +++ b/src/libcamera/pipeline/virtual/virtual.h >>> @@ -66,6 +66,7 @@ public: >>> >>> std::vector<StreamConfig> streamConfigs_; >>> Signal<FrameBuffer *> bufferCompleted; >>> + uint64_t lastTimestamp = 0; >>> }; >>> >>> } /* namespace libcamera */ >>> -- >>> 2.56.0 >>>
Quoting Robert Mader (2026-10-13 12:16:43) > Hi > > On 13.10.26 12:36, Kieran Bingham wrote: > > Quoting Kieran Bingham (2026-10-13 11:34:07) > >> Quoting Robert Mader (2026-10-13 11:22:56) > >>> Throttle requests to the minimal frame duration in order to honor the > >>> configured framerates. Client-requested values are ignored for now. > >>> > >>> Signed-off-by: Robert Mader <robert.mader@collabora.com> > >>> > >>> --- > >>> > >>> Changes in v2: > >>> - Remove debug log in order to avoid CI issues. > >>> - Simplify code in queueRequestDevice(), hopefully making it easier to > >>> follow. > >>> > >>> Changes in v1 (from RFC version): > >>> - Use std::this_thread::sleep_for() instead usleep() (Barnabás) > >>> - Sleep only once per request, after generating all frames > >>> - Remove control handling in start() (defer that for later) > >>> - Reset state in stop() instead of start() > >>> --- > >>> src/libcamera/pipeline/virtual/virtual.cpp | 42 ++++++++++++++++++---- > >>> src/libcamera/pipeline/virtual/virtual.h | 1 + > >>> 2 files changed, 37 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > >>> index e25dd85a7391..5f00f7cc6b37 100644 > >>> --- a/src/libcamera/pipeline/virtual/virtual.cpp > >>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp > >>> @@ -130,6 +130,9 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe, > >>> > >>> void VirtualCameraData::processRequest(Request *request) > >>> { > >>> + int64_t sensorTimestamp = *request->metadata().get(controls::SensorTimestamp); > >>> + std::vector<FrameBuffer *> buffers; > >>> + > >>> for (const auto &[stream, buffer] : request->buffers()) { > >>> bool found = false; > >>> /* map buffer and fill test patterns */ > >>> @@ -139,7 +142,7 @@ void VirtualCameraData::processRequest(Request *request) > >>> > >>> fmd.status = FrameMetadata::Status::FrameSuccess; > >>> fmd.sequence = streamConfig.seq++; > >>> - fmd.timestamp = currentTimestamp(); > >>> + fmd.timestamp = sensorTimestamp; > >>> > >>> std::span<const FrameBuffer::Plane> planes = buffer->planes(); > >>> for (const auto [i, p] : utils::enumerate(planes)) > >>> @@ -151,12 +154,21 @@ void VirtualCameraData::processRequest(Request *request) > >>> stream->configuration().size, buffer)) > >>> fmd.status = FrameMetadata::Status::FrameError; > >>> > >>> - bufferCompleted.emit(buffer); > >>> + buffers.push_back(buffer); > >>> break; > >>> } > >>> } > >>> ASSERT(found); > >>> } > >>> + > >>> + const uint64_t now = currentTimestamp(); > >>> + if (now < static_cast<uint64_t>(sensorTimestamp)) { > >>> + std::chrono::nanoseconds duration(sensorTimestamp - now); > >>> + std::this_thread::sleep_for(duration); > >>> + } > >>> + > >>> + for (auto buffer : buffers) > >>> + bufferCompleted.emit(buffer); > >>> } > >>> > >>> VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data) > >>> @@ -349,18 +361,36 @@ void PipelineHandlerVirtual::stopDevice(Camera *camera) > >>> /* Process pending `bufferCompleted` signals. */ > >>> thread()->dispatchMessages(Message::Type::InvokeMessage, this); > >>> data->bufferCompleted.disconnect(this); > >>> + data->lastTimestamp = 0; > >>> > >>> while (!data->queuedRequests_.empty()) > >>> cancelRequest(data->queuedRequests_.front()); > >>> } > >>> > >>> -int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > >>> - Request *request) > >>> +int PipelineHandlerVirtual::queueRequestDevice(Camera *camera, Request *request) > >>> { > >>> VirtualCameraData *data = cameraData(camera); > >>> - const auto timestamp = currentTimestamp(); > >>> + const uint64_t now = currentTimestamp(); > >>> + uint64_t minFrameDurationNsec; > >>> + uint64_t targetTimestamp; > >>> + > >>> + auto frameDurationLimits = camera->controls().find(&controls::FrameDurationLimits); > >>> + if (false && frameDurationLimits != camera->controls().end()) { > >>> + minFrameDurationNsec = > >>> + frameDurationLimits->second.min().get<int64_t>() * 1000; > >>> + } else { > >>> + /* Default to 30 FPS */ > >>> + minFrameDurationNsec = 33333 * 1000; > >>> + } > >> Something here can't be right. > >> > >> The camera->controls() is a list of the controls the camera supports, > >> not what the application has asked for. > >> > >> The request brings in what applications ask for - i.e. to set/change the > >> FrameDurationLimits, so either the FrameDurationLimits control is always > >> there or ... > >> > >> Wait - now I see you have "if (false &&" so you already know there's > >> dead code there. > >> > >> Lets not add dead code ... espcially dead code which is > >> incorrect/misleading when/if enabled. > Yeah, sorry, fixed that in v3 - the left-over false was from testing the > else path, however I think we can't ever hit it - the config parser > should always set some value. So I should probably just put an assert here. > >> > >> I don't think you need to add an IPA here, but if you want to be able to > >> process FrameDurationLimits, you need to register it as a control in the > >> info map with limits, and during queueRequest you can parse it and > >> update a local frameDuration variable. > >> > >> That's then something you can use to determine the next timestamp. > > I see you did mention that client requests aren't handled yet, so if > > that's the case, lets not add the code. But I think you could handle > > them easily. > I would like to do so, however I both wasn't sure how to do it (do I > need to re-create the control map? Or can I just somehow set the value?) In any form that works - add it to the ControlInfoMap ... > and how to test it (I guess with camshark?). Going forward I'd like to > see the control handled by the Pipewire plugin as well. Either way, I'd > prefer to do so as follow-up. Yes, camshark is always the easiest thing I find to test things. > > But also - either way, you should set the FrameDuration metadata. > You mean to the request metadata, just like we already do for > controls::SensorTimestamp? Sure, can do so. Yes please :D - it should be easy! > > > >> -- > >> Kieran > >> > >> > >> > >>> + > >>> + targetTimestamp = data->lastTimestamp + minFrameDurationNsec; > >>> + if (targetTimestamp < now) > >>> + targetTimestamp = now; > >>> + data->lastTimestamp = targetTimestamp; > >>> + > >>> + request->_d()->metadata().set(controls::SensorTimestamp, > >>> + targetTimestamp); > >>> > >>> - request->_d()->metadata().set(controls::SensorTimestamp, timestamp); > >>> data->invokeMethod(&VirtualCameraData::processRequest, > >>> ConnectionTypeQueued, request); > >>> > >>> diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h > >>> index 215e56fa3b9d..84b53dfb66f3 100644 > >>> --- a/src/libcamera/pipeline/virtual/virtual.h > >>> +++ b/src/libcamera/pipeline/virtual/virtual.h > >>> @@ -66,6 +66,7 @@ public: > >>> > >>> std::vector<StreamConfig> streamConfigs_; > >>> Signal<FrameBuffer *> bufferCompleted; > >>> + uint64_t lastTimestamp = 0; > >>> }; > >>> > >>> } /* namespace libcamera */ > >>> -- > >>> 2.56.0 > >>>
diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp index e25dd85a7391..5f00f7cc6b37 100644 --- a/src/libcamera/pipeline/virtual/virtual.cpp +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -130,6 +130,9 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe, void VirtualCameraData::processRequest(Request *request) { + int64_t sensorTimestamp = *request->metadata().get(controls::SensorTimestamp); + std::vector<FrameBuffer *> buffers; + for (const auto &[stream, buffer] : request->buffers()) { bool found = false; /* map buffer and fill test patterns */ @@ -139,7 +142,7 @@ void VirtualCameraData::processRequest(Request *request) fmd.status = FrameMetadata::Status::FrameSuccess; fmd.sequence = streamConfig.seq++; - fmd.timestamp = currentTimestamp(); + fmd.timestamp = sensorTimestamp; std::span<const FrameBuffer::Plane> planes = buffer->planes(); for (const auto [i, p] : utils::enumerate(planes)) @@ -151,12 +154,21 @@ void VirtualCameraData::processRequest(Request *request) stream->configuration().size, buffer)) fmd.status = FrameMetadata::Status::FrameError; - bufferCompleted.emit(buffer); + buffers.push_back(buffer); break; } } ASSERT(found); } + + const uint64_t now = currentTimestamp(); + if (now < static_cast<uint64_t>(sensorTimestamp)) { + std::chrono::nanoseconds duration(sensorTimestamp - now); + std::this_thread::sleep_for(duration); + } + + for (auto buffer : buffers) + bufferCompleted.emit(buffer); } VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data) @@ -349,18 +361,36 @@ void PipelineHandlerVirtual::stopDevice(Camera *camera) /* Process pending `bufferCompleted` signals. */ thread()->dispatchMessages(Message::Type::InvokeMessage, this); data->bufferCompleted.disconnect(this); + data->lastTimestamp = 0; while (!data->queuedRequests_.empty()) cancelRequest(data->queuedRequests_.front()); } -int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, - Request *request) +int PipelineHandlerVirtual::queueRequestDevice(Camera *camera, Request *request) { VirtualCameraData *data = cameraData(camera); - const auto timestamp = currentTimestamp(); + const uint64_t now = currentTimestamp(); + uint64_t minFrameDurationNsec; + uint64_t targetTimestamp; + + auto frameDurationLimits = camera->controls().find(&controls::FrameDurationLimits); + if (false && frameDurationLimits != camera->controls().end()) { + minFrameDurationNsec = + frameDurationLimits->second.min().get<int64_t>() * 1000; + } else { + /* Default to 30 FPS */ + minFrameDurationNsec = 33333 * 1000; + } + + targetTimestamp = data->lastTimestamp + minFrameDurationNsec; + if (targetTimestamp < now) + targetTimestamp = now; + data->lastTimestamp = targetTimestamp; + + request->_d()->metadata().set(controls::SensorTimestamp, + targetTimestamp); - request->_d()->metadata().set(controls::SensorTimestamp, timestamp); data->invokeMethod(&VirtualCameraData::processRequest, ConnectionTypeQueued, request); diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h index 215e56fa3b9d..84b53dfb66f3 100644 --- a/src/libcamera/pipeline/virtual/virtual.h +++ b/src/libcamera/pipeline/virtual/virtual.h @@ -66,6 +66,7 @@ public: std::vector<StreamConfig> streamConfigs_; Signal<FrameBuffer *> bufferCompleted; + uint64_t lastTimestamp = 0; }; } /* namespace libcamera */
Throttle requests to the minimal frame duration in order to honor the configured framerates. Client-requested values are ignored for now. Signed-off-by: Robert Mader <robert.mader@collabora.com> --- Changes in v2: - Remove debug log in order to avoid CI issues. - Simplify code in queueRequestDevice(), hopefully making it easier to follow. Changes in v1 (from RFC version): - Use std::this_thread::sleep_for() instead usleep() (Barnabás) - Sleep only once per request, after generating all frames - Remove control handling in start() (defer that for later) - Reset state in stop() instead of start() --- src/libcamera/pipeline/virtual/virtual.cpp | 42 ++++++++++++++++++---- src/libcamera/pipeline/virtual/virtual.h | 1 + 2 files changed, 37 insertions(+), 6 deletions(-)