| Message ID | 20250701090630.2654208-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 6b5cc1c92a33704e23bab2e5da699fa1d0678803 |
| Headers | show |
| Series |
|
| Related | show |
2025. 07. 01. 11:06 keltezéssel, Barnabás Pőcze írta: > Process the control list passed to `Camera::start()`, and set > the V4L2 controls accordingly. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- changes in v2: * process controls before streamOn() v1: https://patchwork.libcamera.org/patch/23543/ > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 27 +++++++++++++------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index d52b88042..4b5816dfd 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -100,7 +100,7 @@ public: > private: > int processControl(const UVCCameraData *data, ControlList *controls, > unsigned int id, const ControlValue &value); > - int processControls(UVCCameraData *data, Request *request); > + int processControls(UVCCameraData *data, const ControlList &reqControls); > > bool acquireDevice(Camera *camera) override; > void releaseDevice(Camera *camera) override; > @@ -287,7 +287,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, > return data->video_->exportBuffers(count, buffers); > } > > -int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls) > +int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) > { > UVCCameraData *data = cameraData(camera); > unsigned int count = data->stream_.configuration().bufferCount; > @@ -296,13 +296,22 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList > if (ret < 0) > return ret; > > - ret = data->video_->streamOn(); > - if (ret < 0) { > - data->video_->releaseBuffers(); > - return ret; > + if (controls) { > + ret = processControls(data, *controls); > + if (ret < 0) > + goto err_release_buffers; > } > > + ret = data->video_->streamOn(); > + if (ret < 0) > + goto err_release_buffers; > + > return 0; > + > +err_release_buffers: > + data->video_->releaseBuffers(); > + > + return ret; > } > > void PipelineHandlerUVC::stopDevice(Camera *camera) > @@ -412,11 +421,11 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c > return 0; > } > > -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > +int PipelineHandlerUVC::processControls(UVCCameraData *data, const ControlList &reqControls) > { > ControlList controls(data->video_->controls()); > > - for (const auto &[id, value] : request->controls()) > + for (const auto &[id, value] : reqControls) > processControl(data, &controls, id, value); > > for (const auto &ctrl : controls) > @@ -444,7 +453,7 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request) > return -ENOENT; > } > > - int ret = processControls(data, request); > + int ret = processControls(data, request->controls()); > if (ret < 0) > return ret; >
Quoting Barnabás Pőcze (2025-07-01 10:06:30) > Process the control list passed to `Camera::start()`, and set > the V4L2 controls accordingly. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 27 +++++++++++++------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index d52b88042..4b5816dfd 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -100,7 +100,7 @@ public: > private: > int processControl(const UVCCameraData *data, ControlList *controls, > unsigned int id, const ControlValue &value); > - int processControls(UVCCameraData *data, Request *request); > + int processControls(UVCCameraData *data, const ControlList &reqControls); > > bool acquireDevice(Camera *camera) override; > void releaseDevice(Camera *camera) override; > @@ -287,7 +287,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, > return data->video_->exportBuffers(count, buffers); > } > > -int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls) > +int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) > { > UVCCameraData *data = cameraData(camera); > unsigned int count = data->stream_.configuration().bufferCount; > @@ -296,13 +296,22 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList > if (ret < 0) > return ret; > > - ret = data->video_->streamOn(); > - if (ret < 0) { > - data->video_->releaseBuffers(); > - return ret; > + if (controls) { > + ret = processControls(data, *controls); > + if (ret < 0) > + goto err_release_buffers; > } > Thanks, that ordering is better I think. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + ret = data->video_->streamOn(); > + if (ret < 0) > + goto err_release_buffers; > + > return 0; > + > +err_release_buffers: > + data->video_->releaseBuffers(); > + > + return ret; > } > > void PipelineHandlerUVC::stopDevice(Camera *camera) > @@ -412,11 +421,11 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c > return 0; > } > > -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > +int PipelineHandlerUVC::processControls(UVCCameraData *data, const ControlList &reqControls) > { > ControlList controls(data->video_->controls()); > > - for (const auto &[id, value] : request->controls()) > + for (const auto &[id, value] : reqControls) > processControl(data, &controls, id, value); > > for (const auto &ctrl : controls) > @@ -444,7 +453,7 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request) > return -ENOENT; > } > > - int ret = processControls(data, request); > + int ret = processControls(data, request->controls()); > if (ret < 0) > return ret; > > -- > 2.50.0 >
Hi, On Tue, Jul 01, 2025 at 11:07:35AM +0200, Barnabás Pőcze wrote: > 2025. 07. 01. 11:06 keltezéssel, Barnabás Pőcze írta: > > Process the control list passed to `Camera::start()`, and set > > the V4L2 controls accordingly. > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> LGTM > > --- > > changes in v2: > * process controls before streamOn() > > v1: https://patchwork.libcamera.org/patch/23543/ Paul's R-b tag was not collected on v2. Reviewed-by: Umang Jain <uajain@igalia.com> > > > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 27 +++++++++++++------- > > 1 file changed, 18 insertions(+), 9 deletions(-) > > > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > index d52b88042..4b5816dfd 100644 > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > @@ -100,7 +100,7 @@ public: > > private: > > int processControl(const UVCCameraData *data, ControlList *controls, > > unsigned int id, const ControlValue &value); > > - int processControls(UVCCameraData *data, Request *request); > > + int processControls(UVCCameraData *data, const ControlList &reqControls); > > bool acquireDevice(Camera *camera) override; > > void releaseDevice(Camera *camera) override; > > @@ -287,7 +287,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, > > return data->video_->exportBuffers(count, buffers); > > } > > -int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls) > > +int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) > > { > > UVCCameraData *data = cameraData(camera); > > unsigned int count = data->stream_.configuration().bufferCount; > > @@ -296,13 +296,22 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList > > if (ret < 0) > > return ret; > > - ret = data->video_->streamOn(); > > - if (ret < 0) { > > - data->video_->releaseBuffers(); > > - return ret; > > + if (controls) { > > + ret = processControls(data, *controls); > > + if (ret < 0) > > + goto err_release_buffers; > > } > > + ret = data->video_->streamOn(); > > + if (ret < 0) > > + goto err_release_buffers; > > + > > return 0; > > + > > +err_release_buffers: > > + data->video_->releaseBuffers(); > > + > > + return ret; > > } > > void PipelineHandlerUVC::stopDevice(Camera *camera) > > @@ -412,11 +421,11 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c > > return 0; > > } > > -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > > +int PipelineHandlerUVC::processControls(UVCCameraData *data, const ControlList &reqControls) > > { > > ControlList controls(data->video_->controls()); > > - for (const auto &[id, value] : request->controls()) > > + for (const auto &[id, value] : reqControls) > > processControl(data, &controls, id, value); > > for (const auto &ctrl : controls) > > @@ -444,7 +453,7 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request) > > return -ENOENT; > > } > > - int ret = processControls(data, request); > > + int ret = processControls(data, request->controls()); > > if (ret < 0) > > return ret; >
2025. 07. 01. 12:49 keltezéssel, Umang Jain írta: > Hi, > > On Tue, Jul 01, 2025 at 11:07:35AM +0200, Barnabás Pőcze wrote: >> 2025. 07. 01. 11:06 keltezéssel, Barnabás Pőcze írta: >>> Process the control list passed to `Camera::start()`, and set >>> the V4L2 controls accordingly. >>> >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > LGTM > >>> --- >> >> changes in v2: >> * process controls before streamOn() >> >> v1: https://patchwork.libcamera.org/patch/23543/ > > Paul's R-b tag was not collected on v2. Hmm... I thought the changes were significant enough to omit it; I like to err on the side of caution, but maybe they aren't as significant as I had first thought. > > Reviewed-by: Umang Jain <uajain@igalia.com> >> >> >>> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 27 +++++++++++++------- >>> 1 file changed, 18 insertions(+), 9 deletions(-) >>> >>> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >>> index d52b88042..4b5816dfd 100644 >>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >>> @@ -100,7 +100,7 @@ public: >>> private: >>> int processControl(const UVCCameraData *data, ControlList *controls, >>> unsigned int id, const ControlValue &value); >>> - int processControls(UVCCameraData *data, Request *request); >>> + int processControls(UVCCameraData *data, const ControlList &reqControls); >>> bool acquireDevice(Camera *camera) override; >>> void releaseDevice(Camera *camera) override; >>> @@ -287,7 +287,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, >>> return data->video_->exportBuffers(count, buffers); >>> } >>> -int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls) >>> +int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) >>> { >>> UVCCameraData *data = cameraData(camera); >>> unsigned int count = data->stream_.configuration().bufferCount; >>> @@ -296,13 +296,22 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList >>> if (ret < 0) >>> return ret; >>> - ret = data->video_->streamOn(); >>> - if (ret < 0) { >>> - data->video_->releaseBuffers(); >>> - return ret; >>> + if (controls) { >>> + ret = processControls(data, *controls); >>> + if (ret < 0) >>> + goto err_release_buffers; >>> } >>> + ret = data->video_->streamOn(); >>> + if (ret < 0) >>> + goto err_release_buffers; >>> + >>> return 0; >>> + >>> +err_release_buffers: >>> + data->video_->releaseBuffers(); >>> + >>> + return ret; >>> } >>> void PipelineHandlerUVC::stopDevice(Camera *camera) >>> @@ -412,11 +421,11 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c >>> return 0; >>> } >>> -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) >>> +int PipelineHandlerUVC::processControls(UVCCameraData *data, const ControlList &reqControls) >>> { >>> ControlList controls(data->video_->controls()); >>> - for (const auto &[id, value] : request->controls()) >>> + for (const auto &[id, value] : reqControls) >>> processControl(data, &controls, id, value); >>> for (const auto &ctrl : controls) >>> @@ -444,7 +453,7 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request) >>> return -ENOENT; >>> } >>> - int ret = processControls(data, request); >>> + int ret = processControls(data, request->controls()); >>> if (ret < 0) >>> return ret; >>
Quoting Barnabás Pőcze (2025-07-01 20:57:41) > 2025. 07. 01. 12:49 keltezéssel, Umang Jain írta: > > Hi, > > > > On Tue, Jul 01, 2025 at 11:07:35AM +0200, Barnabás Pőcze wrote: > >> 2025. 07. 01. 11:06 keltezéssel, Barnabás Pőcze írta: > >>> Process the control list passed to `Camera::start()`, and set > >>> the V4L2 controls accordingly. > >>> > >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > > LGTM > > > >>> --- > >> > >> changes in v2: > >> * process controls before streamOn() > >> > >> v1: https://patchwork.libcamera.org/patch/23543/ > > > > Paul's R-b tag was not collected on v2. > > Hmm... I thought the changes were significant enough to omit it; > I like to err on the side of caution, but maybe they aren't as I feel you. > significant as I had first thought. I think it's close enough. If it makes you feel better I can tag it again :) Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > > > > > Reviewed-by: Umang Jain <uajain@igalia.com> > >> > >> > >>> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 27 +++++++++++++------- > >>> 1 file changed, 18 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > >>> index d52b88042..4b5816dfd 100644 > >>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > >>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > >>> @@ -100,7 +100,7 @@ public: > >>> private: > >>> int processControl(const UVCCameraData *data, ControlList *controls, > >>> unsigned int id, const ControlValue &value); > >>> - int processControls(UVCCameraData *data, Request *request); > >>> + int processControls(UVCCameraData *data, const ControlList &reqControls); > >>> bool acquireDevice(Camera *camera) override; > >>> void releaseDevice(Camera *camera) override; > >>> @@ -287,7 +287,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, > >>> return data->video_->exportBuffers(count, buffers); > >>> } > >>> -int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls) > >>> +int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) > >>> { > >>> UVCCameraData *data = cameraData(camera); > >>> unsigned int count = data->stream_.configuration().bufferCount; > >>> @@ -296,13 +296,22 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList > >>> if (ret < 0) > >>> return ret; > >>> - ret = data->video_->streamOn(); > >>> - if (ret < 0) { > >>> - data->video_->releaseBuffers(); > >>> - return ret; > >>> + if (controls) { > >>> + ret = processControls(data, *controls); > >>> + if (ret < 0) > >>> + goto err_release_buffers; > >>> } > >>> + ret = data->video_->streamOn(); > >>> + if (ret < 0) > >>> + goto err_release_buffers; > >>> + > >>> return 0; > >>> + > >>> +err_release_buffers: > >>> + data->video_->releaseBuffers(); > >>> + > >>> + return ret; > >>> } > >>> void PipelineHandlerUVC::stopDevice(Camera *camera) > >>> @@ -412,11 +421,11 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c > >>> return 0; > >>> } > >>> -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > >>> +int PipelineHandlerUVC::processControls(UVCCameraData *data, const ControlList &reqControls) > >>> { > >>> ControlList controls(data->video_->controls()); > >>> - for (const auto &[id, value] : request->controls()) > >>> + for (const auto &[id, value] : reqControls) > >>> processControl(data, &controls, id, value); > >>> for (const auto &ctrl : controls) > >>> @@ -444,7 +453,7 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request) > >>> return -ENOENT; > >>> } > >>> - int ret = processControls(data, request); > >>> + int ret = processControls(data, request->controls()); > >>> if (ret < 0) > >>> return ret; > >> >
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index d52b88042..4b5816dfd 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -100,7 +100,7 @@ public: private: int processControl(const UVCCameraData *data, ControlList *controls, unsigned int id, const ControlValue &value); - int processControls(UVCCameraData *data, Request *request); + int processControls(UVCCameraData *data, const ControlList &reqControls); bool acquireDevice(Camera *camera) override; void releaseDevice(Camera *camera) override; @@ -287,7 +287,7 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, return data->video_->exportBuffers(count, buffers); } -int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls) +int PipelineHandlerUVC::start(Camera *camera, const ControlList *controls) { UVCCameraData *data = cameraData(camera); unsigned int count = data->stream_.configuration().bufferCount; @@ -296,13 +296,22 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList if (ret < 0) return ret; - ret = data->video_->streamOn(); - if (ret < 0) { - data->video_->releaseBuffers(); - return ret; + if (controls) { + ret = processControls(data, *controls); + if (ret < 0) + goto err_release_buffers; } + ret = data->video_->streamOn(); + if (ret < 0) + goto err_release_buffers; + return 0; + +err_release_buffers: + data->video_->releaseBuffers(); + + return ret; } void PipelineHandlerUVC::stopDevice(Camera *camera) @@ -412,11 +421,11 @@ int PipelineHandlerUVC::processControl(const UVCCameraData *data, ControlList *c return 0; } -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) +int PipelineHandlerUVC::processControls(UVCCameraData *data, const ControlList &reqControls) { ControlList controls(data->video_->controls()); - for (const auto &[id, value] : request->controls()) + for (const auto &[id, value] : reqControls) processControl(data, &controls, id, value); for (const auto &ctrl : controls) @@ -444,7 +453,7 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request) return -ENOENT; } - int ret = processControls(data, request); + int ret = processControls(data, request->controls()); if (ret < 0) return ret;
Process the control list passed to `Camera::start()`, and set the V4L2 controls accordingly. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 27 +++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-)