| Message ID | 20251210133704.2711629-2-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás On Wed, Dec 10, 2025 at 02:37:03PM +0100, Barnabás Pőcze wrote: > If available, query the time-per-frame parameter from the device after > starting, and report it in the metadata. > > The reported frame duration remains constant during a particular streaming > session, and will most likely not accurately reflect the actual frame > duration for any given frame, depending on the manual exposure time, or if > `V4L2_CID_EXPOSURE_AUTO_PRIORITY` or similar is in effect. But at least it > shows the "intended" frame duration. Could the value be refetched during streaming ? > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > include/libcamera/internal/v4l2_videodevice.h | 2 + > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 13 +++++ > src/libcamera/v4l2_videodevice.cpp | 55 +++++++++++++++++++ I would split the change in two, one for vdev and one for uvc > 3 files changed, 70 insertions(+) > > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h > index 57db0036db..82d98184ed 100644 > --- a/include/libcamera/internal/v4l2_videodevice.h > +++ b/include/libcamera/internal/v4l2_videodevice.h > @@ -208,6 +208,8 @@ public: > int setFormat(V4L2DeviceFormat *format); > Formats formats(uint32_t code = 0); > > + int getFrameInterval(std::chrono::microseconds *interval); > + > int getSelection(unsigned int target, Rectangle *rect); > int setSelection(unsigned int target, Rectangle *rect); > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index cb8cc82dff..3f98e8ece0 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -60,6 +60,7 @@ public: > > std::optional<v4l2_exposure_auto_type> autoExposureMode_; > std::optional<v4l2_exposure_auto_type> manualExposureMode_; > + std::optional<std::chrono::microseconds> timePerFrame_; > > private: > bool generateId(); > @@ -295,6 +296,8 @@ int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) > UVCCameraData *data = cameraData(camera); > unsigned int count = data->stream_.configuration().bufferCount; > > + data->timePerFrame_.reset(); > + > int ret = data->video_->importBuffers(count); > if (ret < 0) > return ret; > @@ -309,6 +312,13 @@ int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) > if (ret < 0) > goto err_release_buffers; > > + if (!data->timePerFrame_) { Haven't you just reset it here above ? > + std::chrono::microseconds interval; > + ret = data->video_->getFrameInterval(&interval); > + if (ret == 0) > + data->timePerFrame_ = interval; > + } > + > return 0; > > err_release_buffers: > @@ -898,6 +908,9 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer) > request->metadata().set(controls::SensorTimestamp, > buffer->metadata().timestamp); > > + if (timePerFrame_) > + request->metadata().set(controls::FrameDuration, timePerFrame_->count()); > + > pipe()->completeBuffer(request, buffer); > pipe()->completeRequest(request); > } > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 25b61d049a..3836dabef3 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -1147,6 +1147,61 @@ V4L2VideoDevice::Formats V4L2VideoDevice::formats(uint32_t code) > return formats; > } > > +namespace { > + > +std::chrono::microseconds > +v4l2FractionToMs(const v4l2_fract &f) > +{ > + auto seconds = std::chrono::duration<float>(f.numerator) / f.denominator; > + return std::chrono::duration_cast<std::chrono::microseconds>(seconds); > +} > + > +} > + > +/** > + * \brief Retrieve the frame interval set on the V4L2 video device I think it would be nice to specify the unit in the function documentation (microseconds) > + * \param[out] interval The frame interval applied on the device > + * > + * Retrieve the current time-per-frame parameter from the device. > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int V4L2VideoDevice::getFrameInterval(std::chrono::microseconds *interval) > +{ > + const v4l2_fract *frameInterval = nullptr; > + v4l2_streamparm sparm = {}; > + uint32_t caps = 0; > + > + sparm.type = bufferType_; > + > + int ret = ioctl(VIDIOC_G_PARM, &sparm); Now, this really is not an area I'm expert, so better check with Kieran and Laurent on UVC, but to me G_PARM has always been a no-no. Probably because I always had to deal with complex devices where the duration wasn't set/retrieved from the video device but rather from other entities on the media graph. Maybe it is still legit for UVC and other videodev-centric platforms ? > + if (ret) > + return ret; For most if not all devices supported by libcamera, this will return -ENOTTY. In other words, I do expect uvc to be the only valid user of this function. Do you think it's a concern ? > + > + switch (sparm.type) { > + case V4L2_BUF_TYPE_VIDEO_CAPTURE: > + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: > + frameInterval = &sparm.parm.capture.timeperframe; > + caps = sparm.parm.capture.capability; > + break; > + case V4L2_BUF_TYPE_VIDEO_OUTPUT: > + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: > + frameInterval = &sparm.parm.output.timeperframe; > + caps = sparm.parm.output.capability; > + break; > + } > + > + if (!frameInterval) > + return -EINVAL; > + > + if (!(caps & V4L2_CAP_TIMEPERFRAME)) > + return -ENOTSUP; Should we check this before even trying to ioctl() or is this only to signal to userspace that time-per-frame can be changed ? I read in Documentation/userspace-api/media/v4l/vidioc-g-parm.rst * - ``V4L2_CAP_TIMEPERFRAME`` - 0x1000 - The frame period can be modified by setting the ``timeperframe`` field. > + > + *interval = v4l2FractionToMs(*frameInterval); > + > + return 0; > +} > + > std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats(uint32_t code) > { > std::vector<V4L2PixelFormat> formats; > -- > 2.52.0 >
Hi 2025. 12. 10. 16:39 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Wed, Dec 10, 2025 at 02:37:03PM +0100, Barnabás Pőcze wrote: >> If available, query the time-per-frame parameter from the device after >> starting, and report it in the metadata. >> >> The reported frame duration remains constant during a particular streaming >> session, and will most likely not accurately reflect the actual frame >> duration for any given frame, depending on the manual exposure time, or if >> `V4L2_CID_EXPOSURE_AUTO_PRIORITY` or similar is in effect. But at least it >> shows the "intended" frame duration. > > Could the value be refetched during streaming ? As far as I am aware this parameter is fixed for a given streaming session, so it would return the value that was set. > >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> include/libcamera/internal/v4l2_videodevice.h | 2 + >> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 13 +++++ >> src/libcamera/v4l2_videodevice.cpp | 55 +++++++++++++++++++ > > I would split the change in two, one for vdev and one for uvc I will try. > >> 3 files changed, 70 insertions(+) >> >> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h >> index 57db0036db..82d98184ed 100644 >> --- a/include/libcamera/internal/v4l2_videodevice.h >> +++ b/include/libcamera/internal/v4l2_videodevice.h >> @@ -208,6 +208,8 @@ public: >> int setFormat(V4L2DeviceFormat *format); >> Formats formats(uint32_t code = 0); >> >> + int getFrameInterval(std::chrono::microseconds *interval); >> + >> int getSelection(unsigned int target, Rectangle *rect); >> int setSelection(unsigned int target, Rectangle *rect); >> >> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> index cb8cc82dff..3f98e8ece0 100644 >> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> @@ -60,6 +60,7 @@ public: >> >> std::optional<v4l2_exposure_auto_type> autoExposureMode_; >> std::optional<v4l2_exposure_auto_type> manualExposureMode_; >> + std::optional<std::chrono::microseconds> timePerFrame_; >> >> private: >> bool generateId(); >> @@ -295,6 +296,8 @@ int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) >> UVCCameraData *data = cameraData(camera); >> unsigned int count = data->stream_.configuration().bufferCount; >> >> + data->timePerFrame_.reset(); >> + >> int ret = data->video_->importBuffers(count); >> if (ret < 0) >> return ret; >> @@ -309,6 +312,13 @@ int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) >> if (ret < 0) >> goto err_release_buffers; >> >> + if (!data->timePerFrame_) { > > Haven't you just reset it here above ? That's true, this only makes sense with the next patch. > >> + std::chrono::microseconds interval; >> + ret = data->video_->getFrameInterval(&interval); >> + if (ret == 0) >> + data->timePerFrame_ = interval; >> + } >> + >> return 0; >> >> err_release_buffers: >> @@ -898,6 +908,9 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer) >> request->metadata().set(controls::SensorTimestamp, >> buffer->metadata().timestamp); >> >> + if (timePerFrame_) >> + request->metadata().set(controls::FrameDuration, timePerFrame_->count()); >> + >> pipe()->completeBuffer(request, buffer); >> pipe()->completeRequest(request); >> } >> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp >> index 25b61d049a..3836dabef3 100644 >> --- a/src/libcamera/v4l2_videodevice.cpp >> +++ b/src/libcamera/v4l2_videodevice.cpp >> @@ -1147,6 +1147,61 @@ V4L2VideoDevice::Formats V4L2VideoDevice::formats(uint32_t code) >> return formats; >> } >> >> +namespace { >> + >> +std::chrono::microseconds >> +v4l2FractionToMs(const v4l2_fract &f) >> +{ >> + auto seconds = std::chrono::duration<float>(f.numerator) / f.denominator; >> + return std::chrono::duration_cast<std::chrono::microseconds>(seconds); >> +} >> + >> +} >> + >> +/** >> + * \brief Retrieve the frame interval set on the V4L2 video device > > I think it would be nice to specify the unit in the function > documentation (microseconds) Ok. > >> + * \param[out] interval The frame interval applied on the device >> + * >> + * Retrieve the current time-per-frame parameter from the device. >> + * >> + * \return 0 on success or a negative error code otherwise >> + */ >> +int V4L2VideoDevice::getFrameInterval(std::chrono::microseconds *interval) >> +{ >> + const v4l2_fract *frameInterval = nullptr; >> + v4l2_streamparm sparm = {}; >> + uint32_t caps = 0; >> + >> + sparm.type = bufferType_; >> + >> + int ret = ioctl(VIDIOC_G_PARM, &sparm); > > Now, this really is not an area I'm expert, so better check with > Kieran and Laurent on UVC, but to me G_PARM has always been a no-no. > Probably because I always had to deal with complex devices where the > duration wasn't set/retrieved from the video device but rather from > other entities on the media graph. Maybe it is still legit for UVC and > other videodev-centric platforms ? As far as I'm aware this is the way for UVC cameras. > >> + if (ret) >> + return ret; > > For most if not all devices supported by libcamera, this will return > -ENOTTY. In other words, I do expect uvc to be the only valid user of > this function. Do you think it's a concern ? I'm not too concerned, an argument could be made that this VIDIOC_{G,S}_PARM is a function video devices, so exposing it in the V4L2VideoDevice class does not seem too bad to me. > > >> + >> + switch (sparm.type) { >> + case V4L2_BUF_TYPE_VIDEO_CAPTURE: >> + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: >> + frameInterval = &sparm.parm.capture.timeperframe; >> + caps = sparm.parm.capture.capability; >> + break; >> + case V4L2_BUF_TYPE_VIDEO_OUTPUT: >> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: >> + frameInterval = &sparm.parm.output.timeperframe; >> + caps = sparm.parm.output.capability; >> + break; >> + } >> + >> + if (!frameInterval) >> + return -EINVAL; >> + >> + if (!(caps & V4L2_CAP_TIMEPERFRAME)) >> + return -ENOTSUP; > > Should we check this before even trying to ioctl() or is this only to > signal to userspace that time-per-frame can be changed ? I considered checking this capability in `V4L2VideoDevice::open()` and saving it into a member variable. That could be better I suppose, not sure. > > I read in Documentation/userspace-api/media/v4l/vidioc-g-parm.rst > > * - ``V4L2_CAP_TIMEPERFRAME`` > - 0x1000 > - The frame period can be modified by setting the ``timeperframe`` > field. > >> + >> + *interval = v4l2FractionToMs(*frameInterval); >> + >> + return 0; >> +} >> + >> std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats(uint32_t code) >> { >> std::vector<V4L2PixelFormat> formats; >> -- >> 2.52.0 >>
diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 57db0036db..82d98184ed 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -208,6 +208,8 @@ public: int setFormat(V4L2DeviceFormat *format); Formats formats(uint32_t code = 0); + int getFrameInterval(std::chrono::microseconds *interval); + int getSelection(unsigned int target, Rectangle *rect); int setSelection(unsigned int target, Rectangle *rect); diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index cb8cc82dff..3f98e8ece0 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -60,6 +60,7 @@ public: std::optional<v4l2_exposure_auto_type> autoExposureMode_; std::optional<v4l2_exposure_auto_type> manualExposureMode_; + std::optional<std::chrono::microseconds> timePerFrame_; private: bool generateId(); @@ -295,6 +296,8 @@ int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) UVCCameraData *data = cameraData(camera); unsigned int count = data->stream_.configuration().bufferCount; + data->timePerFrame_.reset(); + int ret = data->video_->importBuffers(count); if (ret < 0) return ret; @@ -309,6 +312,13 @@ int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) if (ret < 0) goto err_release_buffers; + if (!data->timePerFrame_) { + std::chrono::microseconds interval; + ret = data->video_->getFrameInterval(&interval); + if (ret == 0) + data->timePerFrame_ = interval; + } + return 0; err_release_buffers: @@ -898,6 +908,9 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer) request->metadata().set(controls::SensorTimestamp, buffer->metadata().timestamp); + if (timePerFrame_) + request->metadata().set(controls::FrameDuration, timePerFrame_->count()); + pipe()->completeBuffer(request, buffer); pipe()->completeRequest(request); } diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 25b61d049a..3836dabef3 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1147,6 +1147,61 @@ V4L2VideoDevice::Formats V4L2VideoDevice::formats(uint32_t code) return formats; } +namespace { + +std::chrono::microseconds +v4l2FractionToMs(const v4l2_fract &f) +{ + auto seconds = std::chrono::duration<float>(f.numerator) / f.denominator; + return std::chrono::duration_cast<std::chrono::microseconds>(seconds); +} + +} + +/** + * \brief Retrieve the frame interval set on the V4L2 video device + * \param[out] interval The frame interval applied on the device + * + * Retrieve the current time-per-frame parameter from the device. + * + * \return 0 on success or a negative error code otherwise + */ +int V4L2VideoDevice::getFrameInterval(std::chrono::microseconds *interval) +{ + const v4l2_fract *frameInterval = nullptr; + v4l2_streamparm sparm = {}; + uint32_t caps = 0; + + sparm.type = bufferType_; + + int ret = ioctl(VIDIOC_G_PARM, &sparm); + if (ret) + return ret; + + switch (sparm.type) { + case V4L2_BUF_TYPE_VIDEO_CAPTURE: + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + frameInterval = &sparm.parm.capture.timeperframe; + caps = sparm.parm.capture.capability; + break; + case V4L2_BUF_TYPE_VIDEO_OUTPUT: + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + frameInterval = &sparm.parm.output.timeperframe; + caps = sparm.parm.output.capability; + break; + } + + if (!frameInterval) + return -EINVAL; + + if (!(caps & V4L2_CAP_TIMEPERFRAME)) + return -ENOTSUP; + + *interval = v4l2FractionToMs(*frameInterval); + + return 0; +} + std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats(uint32_t code) { std::vector<V4L2PixelFormat> formats;
If available, query the time-per-frame parameter from the device after starting, and report it in the metadata. The reported frame duration remains constant during a particular streaming session, and will most likely not accurately reflect the actual frame duration for any given frame, depending on the manual exposure time, or if `V4L2_CID_EXPOSURE_AUTO_PRIORITY` or similar is in effect. But at least it shows the "intended" frame duration. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- include/libcamera/internal/v4l2_videodevice.h | 2 + src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 13 +++++ src/libcamera/v4l2_videodevice.cpp | 55 +++++++++++++++++++ 3 files changed, 70 insertions(+)