| Message ID | 20261026190858.841063-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Barnabás Pőcze (2026-10-26 19:08:57) > From: Robert Mader <robert.mader@collabora.com> > > Throttle requests to the minimal frame duration in order to honor the > configured frame rates. > > Signed-off-by: Robert Mader <robert.mader@collabora.com> Missing an SoB for Barnabás Pőcze <barnabas.pocze@ideasonboard.com>. But Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > > Changes in v8: > - process `FrameDurationLimits` in requests, not just at startup; > the following capture script can be used for testing: > > ``` > properties: > - loop: 500 > frames: > - 0: > FrameDurationLimits: [ 111111, 111111 ] > - 50: > FrameDurationLimits: [ 99999, 99999 ] > - 100: > FrameDurationLimits: [ 88888, 88888 ] > - 150: > FrameDurationLimits: [ 77777, 77777 ] > - 200: > FrameDurationLimits: [ 66666, 66666 ] > - 250: > FrameDurationLimits: [ 55555, 55555 ] > - 300: > FrameDurationLimits: [ 44444, 44444 ] > - 350: > FrameDurationLimits: [ 33333, 33333 ] > - 400: > FrameDurationLimits: [ 22222, 22222 ] > - 450: > FrameDurationLimits: [ 11111, 11111 ] > ``` > > and the configuration has to be modified to accept these values: > > ``` > diff --git a/src/libcamera/pipeline/virtual/data/virtual.yaml b/src/libcamera/pipeline/virtual/data/virtual.yaml > index 20471bb94b..959e83d790 100644 > --- a/src/libcamera/pipeline/virtual/data/virtual.yaml > +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml > @@ -6,8 +6,8 @@ > - width: 1920 > height: 1080 > frame_rates: > - - 30 > - - 60 > + - 1 > + - 120 > - width: 1680 > height: 1050 > frame_rates: > ``` > > Changes in v7: https://patchwork.libcamera.org/patch/28533/ > - Address feedback from Laurent > - Replace lastTimestamp_ with targetTimestamp_ and simplify calculation > > Changes in v6: https://patchwork.libcamera.org/patch/28455/ > - Use std::clamp (Kieran) and minor code reshuffle in start() > - Use "frame rate" consistently > - Added R-B from Kieran (assuming the changes are ok) > > Changes in v5: https://patchwork.libcamera.org/patch/28453/ > - Also clamp frame duration limit to highest one allowed by the config > so clients can't set a unsupported frame rate. > - Added comments for that calculations using frame rate logic, as that is > what the config files use. > - Successfully tested the patch against the following Pipewire MR: > https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/3059 > > Changes in v4: https://patchwork.libcamera.org/patch/28447/ > - Calculate minFrameDuration_ once in start() using FDLs from camera > and, if present, from client provided controls. The later is not yet > tested. > - Assert that the camera has FDLs set by the config parser. > - Set FrameDuration in request metadata. > > Changes in v3: https://patchwork.libcamera.org/patch/28442/ > - Removed a leftover debug change > > Changes in v2: https://patchwork.libcamera.org/patch/28441/ > - 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() > > --- > .../pipeline/virtual/config_parser.cpp | 12 +++- > src/libcamera/pipeline/virtual/virtual.cpp | 61 ++++++++++++++++--- > src/libcamera/pipeline/virtual/virtual.h | 8 ++- > 3 files changed, 68 insertions(+), 13 deletions(-) > > diff --git a/src/libcamera/pipeline/virtual/config_parser.cpp b/src/libcamera/pipeline/virtual/config_parser.cpp > index 5169fd39bc..a18b4213f4 100644 > --- a/src/libcamera/pipeline/virtual/config_parser.cpp > +++ b/src/libcamera/pipeline/virtual/config_parser.cpp > @@ -54,10 +54,16 @@ ConfigParser::parseConfigFile(File &file, PipelineHandler *pipe) > > data->config_.id = cameraId; > ControlInfoMap::Map controls; > + > /* \todo Check which resolution's frame rate to be reported */ > - controls[&controls::FrameDurationLimits] = > - ControlInfo(1000000 / data->config_.resolutions[0].frameRates[1], > - 1000000 / data->config_.resolutions[0].frameRates[0]); > + data->config_.frameDurationLimits = { > + 1000000 / data->config_.resolutions[0].frameRates[1], > + 1000000 / data->config_.resolutions[0].frameRates[0], > + }; > + controls[&controls::FrameDurationLimits] = ControlInfo{ > + data->config_.frameDurationLimits[0], > + data->config_.frameDurationLimits[1], > + }; > > std::vector<ControlValue> supportedFaceDetectModes{ > static_cast<int32_t>(controls::draft::FaceDetectModeOff), > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > index f35af97b10..f50cb49c6d 100644 > --- a/src/libcamera/pipeline/virtual/virtual.cpp > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > @@ -135,8 +135,14 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe, > moveToThread(this); > } > > -void VirtualCameraData::processRequest(Request *request) > +void VirtualCameraData::processRequest(Request *request, > + uint64_t targetTimestamp) > { > + const auto targetTime = std::chrono::steady_clock::time_point{ > + std::chrono::nanoseconds(targetTimestamp) > + }; > + std::this_thread::sleep_until(targetTime); > + > for (const auto &[stream, buffer] : request->buffers()) { > bool found = false; > /* map buffer and fill test patterns */ > @@ -146,7 +152,7 @@ void VirtualCameraData::processRequest(Request *request) > > fmd.status = FrameMetadata::Status::FrameSuccess; > fmd.sequence = streamConfig.seq++; > - fmd.timestamp = currentTimestamp(); > + fmd.timestamp = targetTimestamp; > > std::span<const FrameBuffer::Plane> planes = buffer->planes(); > for (const auto [i, p] : utils::enumerate(planes)) > @@ -166,6 +172,22 @@ void VirtualCameraData::processRequest(Request *request) > } > } > > +void VirtualCameraData::processControls(const ControlList &ctrls) > +{ > + auto fdl = ctrls.get(controls::FrameDurationLimits); > + if (!fdl) > + return; > + > + /* > + * Target the highest frame rate allowed by the client, > + * clamped to the values allowed by the config. > + */ > + targetFrameDuration_ = std::clamp( > + fdl->front(), > + config_.frameDurationLimits[0], > + config_.frameDurationLimits[1]); > +} > + > VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data) > : CameraConfiguration(), data_(data) > { > @@ -348,11 +370,19 @@ int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera *camera, > return dmaBufAllocator_.exportBuffers(config.bufferCount, planeSizes, buffers); > } > > -int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera, > - [[maybe_unused]] const ControlList *controls) > +int PipelineHandlerVirtual::start(Camera *camera, const ControlList *controls) > { > VirtualCameraData *data = cameraData(camera); > > + /* By default target the highest frame rate allowed by the config. */ > + data->targetFrameDuration_ = data->config_.frameDurationLimits[0]; > + ASSERT(data->targetFrameDuration_ > 0); > + > + if (controls) > + data->processControls(*controls); > + > + data->targetTimestamp_ = 0; > + > for (auto &s : data->streamConfigs_) > s.seq = 0; > > @@ -379,15 +409,28 @@ void PipelineHandlerVirtual::stopDevice(Camera *camera) > 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(); > > - request->_d()->metadata().set(controls::SensorTimestamp, timestamp); > + data->processControls(request->controls()); > + > + /* > + * TODO: Skip frames if we can't keep up with the maximum frame > + * duration. > + */ > + data->targetTimestamp_ = std::max( > + currentTimestamp(), > + data->targetTimestamp_ + data->targetFrameDuration_ * 1000); > + > + request->_d()->metadata().set(controls::SensorTimestamp, > + data->targetTimestamp_); > + request->_d()->metadata().set(controls::FrameDuration, > + data->targetFrameDuration_); > + > data->invokeMethod(&VirtualCameraData::processRequest, > - ConnectionTypeQueued, request); > + ConnectionTypeQueued, request, > + data->targetTimestamp_); > > return 0; > } > diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h > index 215e56fa3b..e4a3fd0854 100644 > --- a/src/libcamera/pipeline/virtual/virtual.h > +++ b/src/libcamera/pipeline/virtual/virtual.h > @@ -53,6 +53,8 @@ public: > > Size maxResolutionSize; > Size minResolutionSize; > + > + std::array<int64_t, 2> frameDurationLimits = {}; > }; > > VirtualCameraData(PipelineHandler *pipe, > @@ -60,12 +62,16 @@ public: > > ~VirtualCameraData() = default; > > - void processRequest(Request *request); > + void processRequest(Request *request, uint64_t targetTimestamp); > + void processControls(const ControlList &ctrls); > > Configuration config_; > > std::vector<StreamConfig> streamConfigs_; > Signal<FrameBuffer *> bufferCompleted; > + > + int64_t targetFrameDuration_; > + uint64_t targetTimestamp_; > }; > > } /* namespace libcamera */ > -- > 2.56.0
diff --git a/src/libcamera/pipeline/virtual/data/virtual.yaml b/src/libcamera/pipeline/virtual/data/virtual.yaml index 20471bb94b..959e83d790 100644 --- a/src/libcamera/pipeline/virtual/data/virtual.yaml +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml @@ -6,8 +6,8 @@ - width: 1920 height: 1080 frame_rates: - - 30 - - 60 + - 1 + - 120 - width: 1680 height: 1050 frame_rates: ``` Changes in v7: https://patchwork.libcamera.org/patch/28533/ - Address feedback from Laurent - Replace lastTimestamp_ with targetTimestamp_ and simplify calculation Changes in v6: https://patchwork.libcamera.org/patch/28455/ - Use std::clamp (Kieran) and minor code reshuffle in start() - Use "frame rate" consistently - Added R-B from Kieran (assuming the changes are ok) Changes in v5: https://patchwork.libcamera.org/patch/28453/ - Also clamp frame duration limit to highest one allowed by the config so clients can't set a unsupported frame rate. - Added comments for that calculations using frame rate logic, as that is what the config files use. - Successfully tested the patch against the following Pipewire MR: https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/3059 Changes in v4: https://patchwork.libcamera.org/patch/28447/ - Calculate minFrameDuration_ once in start() using FDLs from camera and, if present, from client provided controls. The later is not yet tested. - Assert that the camera has FDLs set by the config parser. - Set FrameDuration in request metadata. Changes in v3: https://patchwork.libcamera.org/patch/28442/ - Removed a leftover debug change Changes in v2: https://patchwork.libcamera.org/patch/28441/ - 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() --- .../pipeline/virtual/config_parser.cpp | 12 +++- src/libcamera/pipeline/virtual/virtual.cpp | 61 ++++++++++++++++--- src/libcamera/pipeline/virtual/virtual.h | 8 ++- 3 files changed, 68 insertions(+), 13 deletions(-) diff --git a/src/libcamera/pipeline/virtual/config_parser.cpp b/src/libcamera/pipeline/virtual/config_parser.cpp index 5169fd39bc..a18b4213f4 100644 --- a/src/libcamera/pipeline/virtual/config_parser.cpp +++ b/src/libcamera/pipeline/virtual/config_parser.cpp @@ -54,10 +54,16 @@ ConfigParser::parseConfigFile(File &file, PipelineHandler *pipe) data->config_.id = cameraId; ControlInfoMap::Map controls; + /* \todo Check which resolution's frame rate to be reported */ - controls[&controls::FrameDurationLimits] = - ControlInfo(1000000 / data->config_.resolutions[0].frameRates[1], - 1000000 / data->config_.resolutions[0].frameRates[0]); + data->config_.frameDurationLimits = { + 1000000 / data->config_.resolutions[0].frameRates[1], + 1000000 / data->config_.resolutions[0].frameRates[0], + }; + controls[&controls::FrameDurationLimits] = ControlInfo{ + data->config_.frameDurationLimits[0], + data->config_.frameDurationLimits[1], + }; std::vector<ControlValue> supportedFaceDetectModes{ static_cast<int32_t>(controls::draft::FaceDetectModeOff), diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp index f35af97b10..f50cb49c6d 100644 --- a/src/libcamera/pipeline/virtual/virtual.cpp +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -135,8 +135,14 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe, moveToThread(this); } -void VirtualCameraData::processRequest(Request *request) +void VirtualCameraData::processRequest(Request *request, + uint64_t targetTimestamp) { + const auto targetTime = std::chrono::steady_clock::time_point{ + std::chrono::nanoseconds(targetTimestamp) + }; + std::this_thread::sleep_until(targetTime); + for (const auto &[stream, buffer] : request->buffers()) { bool found = false; /* map buffer and fill test patterns */ @@ -146,7 +152,7 @@ void VirtualCameraData::processRequest(Request *request) fmd.status = FrameMetadata::Status::FrameSuccess; fmd.sequence = streamConfig.seq++; - fmd.timestamp = currentTimestamp(); + fmd.timestamp = targetTimestamp; std::span<const FrameBuffer::Plane> planes = buffer->planes(); for (const auto [i, p] : utils::enumerate(planes)) @@ -166,6 +172,22 @@ void VirtualCameraData::processRequest(Request *request) } } +void VirtualCameraData::processControls(const ControlList &ctrls) +{ + auto fdl = ctrls.get(controls::FrameDurationLimits); + if (!fdl) + return; + + /* + * Target the highest frame rate allowed by the client, + * clamped to the values allowed by the config. + */ + targetFrameDuration_ = std::clamp( + fdl->front(), + config_.frameDurationLimits[0], + config_.frameDurationLimits[1]); +} + VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data) : CameraConfiguration(), data_(data) { @@ -348,11 +370,19 @@ int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera *camera, return dmaBufAllocator_.exportBuffers(config.bufferCount, planeSizes, buffers); } -int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera, - [[maybe_unused]] const ControlList *controls) +int PipelineHandlerVirtual::start(Camera *camera, const ControlList *controls) { VirtualCameraData *data = cameraData(camera); + /* By default target the highest frame rate allowed by the config. */ + data->targetFrameDuration_ = data->config_.frameDurationLimits[0]; + ASSERT(data->targetFrameDuration_ > 0); + + if (controls) + data->processControls(*controls); + + data->targetTimestamp_ = 0; + for (auto &s : data->streamConfigs_) s.seq = 0; @@ -379,15 +409,28 @@ void PipelineHandlerVirtual::stopDevice(Camera *camera) 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(); - request->_d()->metadata().set(controls::SensorTimestamp, timestamp); + data->processControls(request->controls()); + + /* + * TODO: Skip frames if we can't keep up with the maximum frame + * duration. + */ + data->targetTimestamp_ = std::max( + currentTimestamp(), + data->targetTimestamp_ + data->targetFrameDuration_ * 1000); + + request->_d()->metadata().set(controls::SensorTimestamp, + data->targetTimestamp_); + request->_d()->metadata().set(controls::FrameDuration, + data->targetFrameDuration_); + data->invokeMethod(&VirtualCameraData::processRequest, - ConnectionTypeQueued, request); + ConnectionTypeQueued, request, + data->targetTimestamp_); return 0; } diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h index 215e56fa3b..e4a3fd0854 100644 --- a/src/libcamera/pipeline/virtual/virtual.h +++ b/src/libcamera/pipeline/virtual/virtual.h @@ -53,6 +53,8 @@ public: Size maxResolutionSize; Size minResolutionSize; + + std::array<int64_t, 2> frameDurationLimits = {}; }; VirtualCameraData(PipelineHandler *pipe, @@ -60,12 +62,16 @@ public: ~VirtualCameraData() = default; - void processRequest(Request *request); + void processRequest(Request *request, uint64_t targetTimestamp); + void processControls(const ControlList &ctrls); Configuration config_; std::vector<StreamConfig> streamConfigs_; Signal<FrameBuffer *> bufferCompleted; + + int64_t targetFrameDuration_; + uint64_t targetTimestamp_; }; } /* namespace libcamera */