Hi Greg, Krzysztof and Nicolas, On Tue, May 13, 2025 at 12:31:49PM +0200, Krzysztof Opasiak wrote:
On 13.05.2025 12:04, Nicolas Dufresne wrote:Hi Greg, Krzysztof, Le mardi 13 mai 2025 à 07:04 +0200, Greg KH a écrit :On Mon, May 12, 2025 at 11:03:41PM +0200, Krzysztof Opasiak wrote:On 12.05.2025 12:43, Krzysztof Opasiak wrote:On 12.05.2025 12:38, Greg KH wrote:On Mon, May 12, 2025 at 12:19:07PM +0200, Krzysztof Opasiak wrote:Hi Greg, On 4.12.2022 09:29, Greg KH wrote:On Sat, Dec 03, 2022 at 11:26:14PM +0200, Laurent Pinchart wrote:Hi Michael, On Sat, Sep 10, 2022 at 12:13:31AM +0200, Michael Grzeschik wrote:[...]Given that I'd like to suggest that it seems to actually make sense to revert this unless there are some ideas how to fix it.Sorry about this, can you submit a patch series that reverts the offending commits? As it was years ago, I don't exactly know what you are referring to anymore.Sure! Will do.Would you prefer to have a set of actual reverts related to this: da692963df4e Revert "usb: gadget: uvc: add v4l2 enumeration api calls" bca75df69aaf Revert "usb: gadget: uvc: add v4l2 try_format api call" e56c767a6d3c Revert "usb: gadget: uvc: also use try_format in set_format" 20f275b86960 Revert "usb: gadget: uvc: fix try format returns on uncompressed formats" 059d98f60c21 Revert "usb: gadget: uvc: Fix ERR_PTR dereference in uvc_v4l2.c" e6fd9b67414c Revert "usb: gadget: webcam: Make g_webcam loadable again" but have a negative consequence that the series isn't really bisectable from functional perspective. For example commit e6fd9b67414c breaks g_uvc until we apply da692963df4e so the series would have to go in as a whole. Or you would prefer a single commit that technically isn't a revert but it just "undoes" the negative consequences of "usb: gadget: uvc: add v4l2 enumeration api calls" (kind of a squash of all commits above)?Ideally we can bisect at all places in the tree, so it's odd that reverting patches would cause problems as when adding them all should have been ok for every commit, right? But if there are merge issues, or other problems, then yes, maybe just one big one is needed, your choice.Won't a plain revert break userspace like GStreamer that have depended on that patch for years ? In such a delicate case, wouldn't it be less damageable to introduce workaround, like alias ? This is one personal script against numerous users of a generic framework implementation.Shouldn't GStreamer be robust enough to handle cases of old-kernel which haven't had this feature at all?The main reason why I reported this is not really the name limitation but the fact that this patch is just incorrect for cases where you would like to expose different formats at different speeds. This feature was there for years and we do have products that depend on it and this change along with the further commits that depend on it broke that support for us.You are absolutely right that those commits added a feature that now someone else may depend thus it would be perfect to fix it in a way that doesn't break anyone's userspace but the problem is that due to the way v4l2 API is designed I really don't see a way how we could make it "speed-aware" without breaking all the users. That's why we are kind of stuck between:1. Leave those commits in and then you cannot different streaming headers for different speeds but users of those API will keep working.2. Revert and bring back the feature of UVC ConfigFS interface that was there since its inception but break the users of "New API".I believe due to the delay, you are facing an unusual ABI breakage, which requires a more delicate handling.Agree. The situation isn't simple as whatever we do would break some userspace... I'm not an expert on v4l2, so if anyone with a better knowledge of v4l2 internals has a suggestion how we could make this work with the existing API I'd be more than happy to try to follow that path.
As a shortcomming compromise I would suggest to support both worlds by conditionally set uvc->header if the directory h was found. If the uvc->header was not set then we could print some info and disable the main part of the possible uvc callbacks that depend on this uvc->header objects. The only downside with that would be that using directory h in the streaming header will implicitly create the limitations of not indipendent formats per speed that Krzysztof mentioned. The alternative would be to put more effort into this and parse all directories in the streaming header subnode. However since the idea of using the v4l2-api is already talked dead by a long discussion history, I would rather focus on transition the remaining functionality of the gstreamer uvcgadget plugin to finally become independent of the v4l2api. How about that transition path? Regards, Michael -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
