On Tue, 17 Dec 2024 at 16:43, Isaac Scott <isaac.scott@xxxxxxxxxxxxxxxx> wrote: > > Hi Ricardo, > > On Tue, 2024-12-17 at 13:08 +0100, Ricardo Ribalda wrote: > > On Tue, 17 Dec 2024 at 12:58, Ricardo Ribalda <ribalda@xxxxxxxxxxxx> > > wrote: > > > > > > Hi Issac > > > > > > > > > On Tue, 17 Dec 2024 at 12:22, Isaac Scott > > > <isaac.scott@xxxxxxxxxxxxxxxx> wrote: > > > > > > > > In use cases where a camera needs to use the > > > > UVC_QUIRK_MJPEG_NO_EOF, > > > > erroneous frames that do not conform to MJPEG standards are > > > > correctly > > > > being marked as erroneous. However, when using the camera in a > > > > product, > > > > manufacturers usually want to enable the quirk in order to pass > > > > the > > > > buffers into user space. To do this, they have to enable the > > > > uvc_no_drop > > > > parameter. To avoid the extra steps to configure the kernel in > > > > such a > > > > way, enable the no_drop parameter by default to comply with this > > > > use > > > > case. > > > > > > I am not sure what you want to do with this patch. > > > > > > Why can't people simply set the quirk with > > > > > > modprobe uvcvideo quirks=0x20000 > > > > Sorry, I meant > > > > modprobe uvcvideo nodrop=1 > > > > or > > > > echo 1 > /sys/module/uvcvideo/parameters/nodrop > > > > That would also work, absolutely! > > If a user has these cameras, they should always add the no_drop to > avoid losing frames that would otherwise be discarded as they are > erroneous. > > This quirk will always trigger, and it is likely they would want to > have all the frames sent by the camera, while still marking them as > errors they can handle later in user space if they want to. Besides what Laurent is saying: ``` One issue with this is that it becomes impossible for someone to unset the no_drop parameter. ``` There is also the issue that with your current implementation you are changing the module parameter for ALL the cameras probed after this one: I believe that you have to do: - ret = uvc_queue_init(queue, type, !uvc_no_drop_param); + ret = uvc_queue_init(queue, type, !uvc_no_drop_param && + !(dev->quirks & UVC_QUIRK_MJPEG_NO_EOF)) But maybe before that we need to have the discussion about: shall we remove uvc_no_drop_param altogether?. We could start by swapping the default value and see what happens.... > > > > > > > -- > > Ricardo Ribalda > > Best wishes, > > Isaac -- Ricardo Ribalda
