[libcamera-devel] [RFC PATCH 02/12] libcamera: pipeline: uvcvideo: Report control errors

[Kieran Bingham]

Quoting Jacopo Mondi (2022-07-25 08:56:59)
> Hi Kieran
> 
> On Thu, Jul 21, 2022 at 01:13:00PM +0100, Kieran Bingham via libcamera-devel wrote:
> > Report an error when failing to process controls, but still allow the
> > request to process and complete where possible.
> >
> > The Request ControlError flag is raised on the request.
> >
> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=135
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > index 53b2f23ab029..1f282f26bec3 100644
> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > @@ -26,6 +26,7 @@
> >  #include "libcamera/internal/device_enumerator.h"
> >  #include "libcamera/internal/media_device.h"
> >  #include "libcamera/internal/pipeline_handler.h"
> > +#include "libcamera/internal/request.h"
> >  #include "libcamera/internal/sysfs.h"
> >  #include "libcamera/internal/v4l2_videodevice.h"
> >
> > @@ -373,8 +374,10 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)
> >       }
> >
> >       int ret = processControls(data, request);
> > -     if (ret < 0)
> > -             return ret;
> > +     if (ret < 0) {
> > +             LOG(UVC, Error) << "Failed to process controls";
> > +             request->_d()->setErrorFlags(Request::ControlError);
> 
> I wonder if Request::Private::setError() wouldn't be enough (that's
> probably a comment for patch #1).

Do you mean 'drop the word Flags' here too ?

> 
> With the confirmation that this patch introduces a behavioural change,
> as before we returned early (which I presume from the commit message
> the change is intended).

Yes, this is very much intended behavioural change.

 
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks


> 
> Thanks
>   j
> 
> > +     }
> >
> >       ret = data->video_->queueBuffer(buffer);
> >       if (ret < 0)
> > --
> > 2.34.1
> >