On Tue, Dec 17, 2024 at 05:02:43PM +0100, Ricardo Ribalda wrote: > On Tue, 17 Dec 2024 at 16:43, Isaac Scott wrote: > > On Tue, 2024-12-17 at 13:08 +0100, Ricardo Ribalda wrote: > > > On Tue, 17 Dec 2024 at 12:58, Ricardo Ribalda wrote: > > > > On Tue, 17 Dec 2024 at 12:22, Isaac Scott 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.... Unless someone objects, I think swapping the default value and waiting a bit is a good idea. If the universe doesn't collapse immediately, we could later drop the parameter. -- Regards, Laurent Pinchart
