Re: ABI breakage due to "Unsupported formats in TRY_FMT/S_FMT" recommendation
Hi, On 12/29/2012 09:32 PM, Mauro Carvalho Chehab wrote: Agreed. Adding YUYV support at libv4l should be easy. Make that easy-ish. There is the easy and the hardway, the easy way is to add an extra conversion step and then convert yuv420 to yuyv, but TBH that is a stupid thing to do, it potentially looses video resolution in the u and v planes and it is just not very cpu effective. So the right thing to do is to go over all the rawXXX -> rgb + rawXXX -> yuv420 converter function pairs, and add a rawXXX -> yuyv function to them for all supported src formats. Applications that use libv4l will do whatever behavior libv4l does. libv4l has 3 different scenarios here: 1) The app is asking for a format libv4l does not support (iow not rgb24 or yuv420), and the video device only supports some proprietary format (ie many gspca webcams), then libv4l will change the requested format to RGB24, do a try_fmt to the device to get closest width/height and returns that. So in this scenario it behaves as Hans V. has suggested. 2) The app is asking for a format libv4l does not support and the device supports one or more standard formats, then libv4l simple passes on the try_fmt to the device, and returns whatever it returns. This is what will happen with most tv-cards, so using libv4l won't help here! 3) The app is asking for a format that libv4l does support, then libv4l negotiates with the device to find the best src format to convert from. libv4l's negotation code will check both the try_fmt return value, as well as that the resulting format is what it asked for. So it does not care either way... Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ABI breakage due to "Unsupported formats in TRY_FMT/S_FMT" recommendation
Em Sat, 29 Dec 2012 14:52:14 -0500 Devin Heitmueller escreveu: > On Sat, Dec 29, 2012 at 9:23 AM, Mauro Carvalho Chehab > wrote: > > On a tvtime compiled without libv4l, the cx18 driver will fail with the > > current logic, as it doesn't return an error when format doesn't > > match. So, tvtime will fail anyway with 50% of the TV drivers that don't > > support YUYV directly. It will also fail with most cameras, as they're > > generally based on proprietary/bayer formats and/or may not have the > > resolutions that tvtime requires. > > > > That's said, libv4l does format conversion. So, if the logic on libv4l > > is working properly, and as tvtime does upport libv4l upstream, > > no real bug should be seen with tvtime, even if the device doesn't > > support neither UYVY or YUYV. > > Tvtime doesn't use libv4l (and never has), unless you added support > very recently and it's not in the linuxtv.org tree. No, I didn't add. Not sure why I was thinking that support for it was added. > I started to look > into making it use libv4l some months back, but libv4l only supports > providing the video to the app in a few select formats (e.g. RGB > formats and YUV 4:2:0). Tvtime specifically needs the video in YUYV > or UYVY because it does all its overlays directly onto the video > buffer, the deinterlacers expect YUYV, and the XVideo support in the > app currently only does YUYV. > > Changing the app to work with 4:2:0 would mean cleaning up the rats > nest that does all of the above functions - certainly not impossible, > but not trivial either. In fact, it would probably be better to add > the colorspace conversion code to libv4l to support providing YUYV to > the app when it asks for it. Agreed. Adding YUYV support at libv4l should be easy. > > The above also applies to MythTV, except that I'm not sure if MythTV uses > > libv4l. > > It does not. > > There's no doubt that both MythTV and Tvtime could use an overhaul of > their V4L2 code (which became as nasty as it is primarily due to all > the years of the kernel's lack of specified behavior and failure to > enforce consistency across boards). That's not really relevant to the > discussion at hand though, which is about breaking existing > applications (and possibly all the apps other than the two or three > common open source apps I raised as examples). Well, xawtv won't break, even without libv4l. Codes based on it won't likely break either. Applications that use libv4l will do whatever behavior libv4l does. I suspect your couple examples are the most used applications that don't fit into either case. So, in order to make the kernel drivers consistent, we need to know what other applications that don't use libv4l and would behave bad with driver changes at VIDIOC_S_FMT's way to return data to implement what it is at the specs. Then, work on a solution that will work everywhere. We can only touch the drivers afer being sure that no regressions will happen. > I would love to take a half dozen tuner boards of various types and > spend a week cleaning up Myth's code, but frankly I just don't have > the time/energy to take on such a task. > > Devin > -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ABI breakage due to "Unsupported formats in TRY_FMT/S_FMT" recommendation
Em Sat, 29 Dec 2012 12:39:11 -0500 Devin Heitmueller escreveu: > > I suspect that the behavior of returning an error if a pixelformat is not > > supported is a leftover from the V4L1 API (VIDIOCSPICT). When drivers were > > converted to S/TRY_FMT this method of handling unsupported pixelformats was > > probably never changed. And quite a few newer drivers copy-and-pasted that > > method. Drivers like cx18/ivtv that didn't copy-and-paste code looked at the > > API and followed what the API said. > > > > At the moment most TV drivers seem to return an error, whereas for webcams > > it is hit and miss: uvc returned an error (until it was fixed recently), > > gspca didn't. So webcam applications probably do the right thing given the > > behavior of gspca. > > What if we returned an error but still changed the struct to specify > the supported format? That's not what the spec says, but it seems > like that's what is implemented in many drivers today and what many > applications expect to happen. That sounds a very bad idea: when an error rises, applications should not trust on any returned value, except for the returned error code itself. All the other values can be on some random state. This is perhaps the only behavior that are consistent on all ioctls of the media subsystem (and likely on the other ioctl's of the Kernel). > No doubt, this is a mess, and if we had tighter enforcement and better > specs before this stuff went upstream years ago, we wouldn't be in > this situation. But that's not the world we live in, and we have to > deal with where we stand today. Well, something needs to be done at Kernel level, in order to make this ioctl consistent among all drivers. I'm open to proposals. In any case, applications aren't implementing the same logic to handle this ioctl. This should be fixed there, in order to avoid unexpected troubles with different hardware. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ABI breakage due to "Unsupported formats in TRY_FMT/S_FMT" recommendation
On Sat, Dec 29, 2012 at 9:23 AM, Mauro Carvalho Chehab wrote: > On a tvtime compiled without libv4l, the cx18 driver will fail with the > current logic, as it doesn't return an error when format doesn't > match. So, tvtime will fail anyway with 50% of the TV drivers that don't > support YUYV directly. It will also fail with most cameras, as they're > generally based on proprietary/bayer formats and/or may not have the > resolutions that tvtime requires. > > That's said, libv4l does format conversion. So, if the logic on libv4l > is working properly, and as tvtime does upport libv4l upstream, > no real bug should be seen with tvtime, even if the device doesn't > support neither UYVY or YUYV. Tvtime doesn't use libv4l (and never has), unless you added support very recently and it's not in the linuxtv.org tree. I started to look into making it use libv4l some months back, but libv4l only supports providing the video to the app in a few select formats (e.g. RGB formats and YUV 4:2:0). Tvtime specifically needs the video in YUYV or UYVY because it does all its overlays directly onto the video buffer, the deinterlacers expect YUYV, and the XVideo support in the app currently only does YUYV. Changing the app to work with 4:2:0 would mean cleaning up the rats nest that does all of the above functions - certainly not impossible, but not trivial either. In fact, it would probably be better to add the colorspace conversion code to libv4l to support providing YUYV to the app when it asks for it. > The above also applies to MythTV, except that I'm not sure if MythTV uses > libv4l. It does not. There's no doubt that both MythTV and Tvtime could use an overhaul of their V4L2 code (which became as nasty as it is primarily due to all the years of the kernel's lack of specified behavior and failure to enforce consistency across boards). That's not really relevant to the discussion at hand though, which is about breaking existing applications (and possibly all the apps other than the two or three common open source apps I raised as examples). I would love to take a half dozen tuner boards of various types and spend a week cleaning up Myth's code, but frankly I just don't have the time/energy to take on such a task. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ABI breakage due to "Unsupported formats in TRY_FMT/S_FMT" recommendation
On Sat, Dec 29, 2012 at 6:53 AM, Hans Verkuil wrote: > In my opinion these are application bugs, and not ABI breakages. I'm not sure if you saw the email, but Linus seems to disagree with your assertion quite strongly: https://lkml.org/lkml/2012/12/23/75 The change as proposed results in a situation where apps worked fine, user upgrades kernel, and now apps are broken. That sounds a lot like: "If a change results in user programs breaking, it's a bug in the kernel. We never EVER blame the user programs. How hard can this be to understand?" > As Mauro > mentioned, some drivers don't return an error and so would always have failed > with these apps (examples: cx18/ivtv, gspca). Yeah, except uncompressed support is quite new with cx18, ivtv doesn't support uncompressed at all, and gspca is for webcams, not TV. > Do these apps even handle the case where TRY isn't implemented at all by the > driver? (hdpvr) I haven't looked at compressed formats. Tvtime doesn't support them at all, and MythTV uses a completely different code path for compressed capture. MythTV even has special code for the HD-PVR, which presumably was to work around API inconsistencies. > There is nothing in the spec that says that you will get an error if the > pixelformat isn't supported by the driver, in fact it says the opposite: > > "Very simple, inflexible devices may even ignore all input and always return > the default parameters." > > We are not in the business to work around application bugs, IMHO. We can of > course delay making these changes until those applications are fixed. Except those two applications aren't the only two applications in existence which make use of the V4L2 API. Oh, and applications are supposed to continue working unmodified through kernel upgrades. > I suspect that the behavior of returning an error if a pixelformat is not > supported is a leftover from the V4L1 API (VIDIOCSPICT). When drivers were > converted to S/TRY_FMT this method of handling unsupported pixelformats was > probably never changed. And quite a few newer drivers copy-and-pasted that > method. Drivers like cx18/ivtv that didn't copy-and-paste code looked at the > API and followed what the API said. > > At the moment most TV drivers seem to return an error, whereas for webcams > it is hit and miss: uvc returned an error (until it was fixed recently), > gspca didn't. So webcam applications probably do the right thing given the > behavior of gspca. What if we returned an error but still changed the struct to specify the supported format? That's not what the spec says, but it seems like that's what is implemented in many drivers today and what many applications expect to happen. No doubt, this is a mess, and if we had tighter enforcement and better specs before this stuff went upstream years ago, we wouldn't be in this situation. But that's not the world we live in, and we have to deal with where we stand today. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ABI breakage due to "Unsupported formats in TRY_FMT/S_FMT" recommendation
Em Sat, 29 Dec 2012 12:23:34 -0200 Mauro Carvalho Chehab escreveu: > Em Sat, 29 Dec 2012 12:53:45 +0100 > Hans Verkuil escreveu: > > > On Sat December 29 2012 01:27:44 Mauro Carvalho Chehab wrote: > > > Em Fri, 28 Dec 2012 14:52:24 -0500 > > > Devin Heitmueller escreveu: > > > > > > > Hi there, > > > > > > > > So I noticed that one of the "V4L2 ambiguities" discussed at the Media > > > > Workshop relates to expected behavior with TRY_FMT/S_FMT. > ... > > > Well, if applications will break with this change, then we need to revisit > > > the question, and decide otherwise, as it shouldn't break userspace. > > > > > > It should be noticed, however, that currently, some drivers won't > > > return errors when S_FMT/TRY_FMT requests invalid parameters. > > > > > > So, IMHO, what should be done is: > > > 1) to not break userspace; > > > 2) userspace apps should be improved to handle those drivers > > > that have a potential of breaking them; > > > 3) clearly state at the API, and enforce via compliance tool, > > > that all drivers will behave the same. > > > > In my opinion these are application bugs, and not ABI breakages. As Mauro > > mentioned, some drivers don't return an error and so would always have > > failed > > with these apps (examples: cx18/ivtv, gspca). > > It is both an application bug and a potential ABI breakage. > > Hmm... as MythTv and tvtime are meant to be used to watch TV, maybe we can > look on it using a different angle. > ... > The drivers that only support V4L2_PIX_FMT_UYVY seems to be > cx18, sta2x11_vip, au0828 and gspca (kinect, w996xcf). From those, > only cx18 and au0828 are TV drivers. > > On a tvtime compiled without libv4l, the cx18 driver will fail with the > current logic, as it doesn't return an error when format doesn't > match. So, tvtime will fail anyway with 50% of the TV drivers that don't > support YUYV directly. It will also fail with most cameras, as they're > generally based on proprietary/bayer formats and/or may not have the > resolutions that tvtime requires. Not sure if I was clear enough. In summary, what I'm saying is that: 1) userspace apps should be fixed, as they're currently broken for cx18, when libv4l support is disabled; 2) from kernelspace's perspective, it seems that the changes for the above affected drivers need to be postponed. If this bug happens only on tvtime and MythTV, then changes on drivers that don't support UYVY could be done anytime, but a yellow flag rised: we should be sure that other userspace applications and libv4l won't be affected before changing an existing kernel driver, as no regressions are accepted. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ABI breakage due to "Unsupported formats in TRY_FMT/S_FMT" recommendation
Em Sat, 29 Dec 2012 12:53:45 +0100 Hans Verkuil escreveu: > On Sat December 29 2012 01:27:44 Mauro Carvalho Chehab wrote: > > Em Fri, 28 Dec 2012 14:52:24 -0500 > > Devin Heitmueller escreveu: > > > > > Hi there, > > > > > > So I noticed that one of the "V4L2 ambiguities" discussed at the Media > > > Workshop relates to expected behavior with TRY_FMT/S_FMT. ... > > Well, if applications will break with this change, then we need to revisit > > the question, and decide otherwise, as it shouldn't break userspace. > > > > It should be noticed, however, that currently, some drivers won't > > return errors when S_FMT/TRY_FMT requests invalid parameters. > > > > So, IMHO, what should be done is: > > 1) to not break userspace; > > 2) userspace apps should be improved to handle those drivers > > that have a potential of breaking them; > > 3) clearly state at the API, and enforce via compliance tool, > > that all drivers will behave the same. > > In my opinion these are application bugs, and not ABI breakages. As Mauro > mentioned, some drivers don't return an error and so would always have failed > with these apps (examples: cx18/ivtv, gspca). It is both an application bug and a potential ABI breakage. Hmm... as MythTv and tvtime are meant to be used to watch TV, maybe we can look on it using a different angle. tvtime tries first V4L2_PIX_FMT_YUYV. If it fails, it tries V4L2_PIX_FMT_UYVY. The drivers that support YUYV directly are: $ git grep -l V4L2_PIX_FMT_YUYV drivers/media/usb drivers/media/pci drivers/media/pci/bt8xx/bttv-driver.c drivers/media/pci/cx23885/cx23885-video.c drivers/media/pci/cx25821/cx25821-video.c drivers/media/pci/cx88/cx88-video.c drivers/media/pci/meye/meye.c drivers/media/pci/saa7134/saa7134-video.c drivers/media/pci/zoran/zoran_driver.c drivers/media/usb/cx231xx/cx231xx-video.c drivers/media/usb/em28xx/em28xx-video.c drivers/media/usb/gspca/ov534.c drivers/media/usb/gspca/vc032x.c drivers/media/usb/s2255/s2255drv.c drivers/media/usb/stkwebcam/stk-sensor.c drivers/media/usb/stkwebcam/stk-webcam.c drivers/media/usb/tlg2300/pd-video.c drivers/media/usb/tm6000/tm6000-video.c drivers/media/usb/usbvision/usbvision-core.c drivers/media/usb/usbvision/usbvision-video.c drivers/media/usb/uvc/uvc_driver.c $ git grep -l V4L2_PIX_FMT_UYVY drivers/media/usb drivers/media/pci drivers/media/pci/bt8xx/bttv-driver.c drivers/media/pci/cx18/cx18-ioctl.c drivers/media/pci/cx18/cx18-streams.c drivers/media/pci/cx23885/cx23885-video.c drivers/media/pci/cx25821/cx25821-video.c drivers/media/pci/cx88/cx88-video.c drivers/media/pci/saa7134/saa7134-video.c drivers/media/pci/sta2x11/sta2x11_vip.c drivers/media/pci/zoran/zoran_driver.c drivers/media/usb/au0828/au0828-video.c drivers/media/usb/gspca/kinect.c drivers/media/usb/gspca/w996Xcf.c drivers/media/usb/s2255/s2255drv.c drivers/media/usb/stk1160/stk1160-v4l.c drivers/media/usb/stkwebcam/stk-sensor.c drivers/media/usb/stkwebcam/stk-webcam.c drivers/media/usb/tm6000/tm6000-core.c drivers/media/usb/tm6000/tm6000-video.c drivers/media/usb/uvc/uvc_driver.c The drivers that only support V4L2_PIX_FMT_UYVY seems to be cx18, sta2x11_vip, au0828 and gspca (kinect, w996xcf). From those, only cx18 and au0828 are TV drivers. On a tvtime compiled without libv4l, the cx18 driver will fail with the current logic, as it doesn't return an error when format doesn't match. So, tvtime will fail anyway with 50% of the TV drivers that don't support YUYV directly. It will also fail with most cameras, as they're generally based on proprietary/bayer formats and/or may not have the resolutions that tvtime requires. That's said, libv4l does format conversion. So, if the logic on libv4l is working properly, and as tvtime does upport libv4l upstream, no real bug should be seen with tvtime, even if the device doesn't support neither UYVY or YUYV. The above also applies to MythTV, except that I'm not sure if MythTV uses libv4l. > Do these apps even handle the case where TRY isn't implemented at all by the > driver? (hdpvr) I think most apps don't try. > There is nothing in the spec that says that you will get an error if the > pixelformat isn't supported by the driver, in fact it says the opposite: > > "Very simple, inflexible devices may even ignore all input and always return > the default parameters." > > We are not in the business to work around application bugs, IMHO. We can of > course delay making these changes until those applications are fixed. Delay such changes make sense for me. > I suspect that the behavior of returning an error if a pixelformat is not > supported is a leftover from the V4L1 API (VIDIOCSPICT). When drivers were > converted to S/TRY_FMT this method of handling unsupported pixelformats was > probably never changed. And quite a few newer drivers copy-and-pasted that > method. Yes, I suspect so. Yet, we should not break userspace. > Drivers like cx18/ivtv that didn't copy-and-paste code looked at the > API and followed what the AP
Re: ABI breakage due to "Unsupported formats in TRY_FMT/S_FMT" recommendation
On Sat December 29 2012 01:27:44 Mauro Carvalho Chehab wrote: > Em Fri, 28 Dec 2012 14:52:24 -0500 > Devin Heitmueller escreveu: > > > Hi there, > > > > So I noticed that one of the "V4L2 ambiguities" discussed at the Media > > Workshop relates to expected behavior with TRY_FMT/S_FMT. > > Specifically (from > > http://www.linuxtv.org/news.php?entry=2012-12-28.mchehab): > > > > === > > 1.4. Unsupported formats in TRY_FMT/S_FMT > > > > What should a driver return in TRY_FMT/S_FMT if the requested format > > is not supported (possible behaviors include returning the currently > > selected format or a default format). > > The spec says this: "Drivers should not return an error code unless > > the input is ambiguous", but it does not explain what constitutes an > > ambiguous input. In my opinion TRY/S_FMT should never return an error > > other than EINVAL (if the buffer type is unsupported) or EBUSY (for > > S_FMT if streaming is in progress). > > Should we make a recommendation whether the currently selected format > > or a default format should be returned? > > One proposal is to just return a default format if the requested > > pixelformat is unsupported. Returning the currently selected format > > leads to inconsistent results. > > Results: > > TRY_FMT/S_FMT should never return an error when the requested format > > is not supported. Drivers should always return a valid format, > > preferably a format that is as widely supported by applications as > > possible. > > Both TRY_FMT and S_FMT should have the same behaviour. Drivers should > > not return different formats when getting the same input parameters. > > The format returned should be a driver default format if possible > > (stateless behaviour) but can be stateful if needed. > > The API spec should let clear that format retuns may be different when > > different video inputs (or outputs) are selected. > > === > > > > Note that this will cause ABI breakage with existing applications. If > > an application expects an error condition to become aware that the > > requested format was not supported, that application will silently > > think the requested format was valid but in fact the driver is > > returning data in some other format. > > > > Tvtime (one of the more popular apps for watching analog television) > > is one such application that will broken. > > > > http://git.linuxtv.org/tvtime.git/blob/HEAD:/src/videoinput.c#l452 > > > > If YUVY isn't supported but UYVY is (for example, with the Hauppauge > > HVR-950q), the application will think it's doing YUYV when in fact the > > driver is returning UYVY. > > > > MythTV is a little better (it does ultimately store the format > > returned by the driver), however even there it depends on an error > > being returned in order to know to do userland format conversion. > > > > https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythtv/recorders/NuppelVideoRecorder.cpp#L1367 > > > > Now it would be quite simple to change tvtime to use the expected > > behavior, but if backward compatibility with the ABI is of paramount > > importance, then we cannot proceed with this change as proposed. > > Don't misunderstand me - if I were inventing the API today then the > > proposed approach is what I would recommend - but since these parts of > > the ABI are something like ten years old, we have to take into account > > legacy applications. > > Thanks for pointing it! > > (c/c Hans, as he started those discussions) > > Well, if applications will break with this change, then we need to revisit > the question, and decide otherwise, as it shouldn't break userspace. > > It should be noticed, however, that currently, some drivers won't > return errors when S_FMT/TRY_FMT requests invalid parameters. > > So, IMHO, what should be done is: > 1) to not break userspace; > 2) userspace apps should be improved to handle those drivers > that have a potential of breaking them; > 3) clearly state at the API, and enforce via compliance tool, > that all drivers will behave the same. In my opinion these are application bugs, and not ABI breakages. As Mauro mentioned, some drivers don't return an error and so would always have failed with these apps (examples: cx18/ivtv, gspca). Do these apps even handle the case where TRY isn't implemented at all by the driver? (hdpvr) There is nothing in the spec that says that you will get an error if the pixelformat isn't supported by the driver, in fact it says the opposite: "Very simple, inflexible devices may even ignore all input and always return the default parameters." We are not in the business to work around application bugs, IMHO. We can of course delay making these changes until those applications are fixed. I suspect that the behavior of returning an error if a pixelformat is not supported is a leftover from the V4L1 API (VIDIOCSPICT). When drivers were converted to S/TRY_FMT this method of handling unsupported pixelformats was probably never c
Re: ABI breakage due to "Unsupported formats in TRY_FMT/S_FMT" recommendation
Em Fri, 28 Dec 2012 14:52:24 -0500 Devin Heitmueller escreveu: > Hi there, > > So I noticed that one of the "V4L2 ambiguities" discussed at the Media > Workshop relates to expected behavior with TRY_FMT/S_FMT. > Specifically (from > http://www.linuxtv.org/news.php?entry=2012-12-28.mchehab): > > === > 1.4. Unsupported formats in TRY_FMT/S_FMT > > What should a driver return in TRY_FMT/S_FMT if the requested format > is not supported (possible behaviors include returning the currently > selected format or a default format). > The spec says this: "Drivers should not return an error code unless > the input is ambiguous", but it does not explain what constitutes an > ambiguous input. In my opinion TRY/S_FMT should never return an error > other than EINVAL (if the buffer type is unsupported) or EBUSY (for > S_FMT if streaming is in progress). > Should we make a recommendation whether the currently selected format > or a default format should be returned? > One proposal is to just return a default format if the requested > pixelformat is unsupported. Returning the currently selected format > leads to inconsistent results. > Results: > TRY_FMT/S_FMT should never return an error when the requested format > is not supported. Drivers should always return a valid format, > preferably a format that is as widely supported by applications as > possible. > Both TRY_FMT and S_FMT should have the same behaviour. Drivers should > not return different formats when getting the same input parameters. > The format returned should be a driver default format if possible > (stateless behaviour) but can be stateful if needed. > The API spec should let clear that format retuns may be different when > different video inputs (or outputs) are selected. > === > > Note that this will cause ABI breakage with existing applications. If > an application expects an error condition to become aware that the > requested format was not supported, that application will silently > think the requested format was valid but in fact the driver is > returning data in some other format. > > Tvtime (one of the more popular apps for watching analog television) > is one such application that will broken. > > http://git.linuxtv.org/tvtime.git/blob/HEAD:/src/videoinput.c#l452 > > If YUVY isn't supported but UYVY is (for example, with the Hauppauge > HVR-950q), the application will think it's doing YUYV when in fact the > driver is returning UYVY. > > MythTV is a little better (it does ultimately store the format > returned by the driver), however even there it depends on an error > being returned in order to know to do userland format conversion. > > https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythtv/recorders/NuppelVideoRecorder.cpp#L1367 > > Now it would be quite simple to change tvtime to use the expected > behavior, but if backward compatibility with the ABI is of paramount > importance, then we cannot proceed with this change as proposed. > Don't misunderstand me - if I were inventing the API today then the > proposed approach is what I would recommend - but since these parts of > the ABI are something like ten years old, we have to take into account > legacy applications. Thanks for pointing it! (c/c Hans, as he started those discussions) Well, if applications will break with this change, then we need to revisit the question, and decide otherwise, as it shouldn't break userspace. It should be noticed, however, that currently, some drivers won't return errors when S_FMT/TRY_FMT requests invalid parameters. So, IMHO, what should be done is: 1) to not break userspace; 2) userspace apps should be improved to handle those drivers that have a potential of breaking them; 3) clearly state at the API, and enforce via compliance tool, that all drivers will behave the same. > > Discussion is welcome. > > Devin Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
ABI breakage due to "Unsupported formats in TRY_FMT/S_FMT" recommendation
Hi there, So I noticed that one of the "V4L2 ambiguities" discussed at the Media Workshop relates to expected behavior with TRY_FMT/S_FMT. Specifically (from http://www.linuxtv.org/news.php?entry=2012-12-28.mchehab): === 1.4. Unsupported formats in TRY_FMT/S_FMT What should a driver return in TRY_FMT/S_FMT if the requested format is not supported (possible behaviors include returning the currently selected format or a default format). The spec says this: "Drivers should not return an error code unless the input is ambiguous", but it does not explain what constitutes an ambiguous input. In my opinion TRY/S_FMT should never return an error other than EINVAL (if the buffer type is unsupported) or EBUSY (for S_FMT if streaming is in progress). Should we make a recommendation whether the currently selected format or a default format should be returned? One proposal is to just return a default format if the requested pixelformat is unsupported. Returning the currently selected format leads to inconsistent results. Results: TRY_FMT/S_FMT should never return an error when the requested format is not supported. Drivers should always return a valid format, preferably a format that is as widely supported by applications as possible. Both TRY_FMT and S_FMT should have the same behaviour. Drivers should not return different formats when getting the same input parameters. The format returned should be a driver default format if possible (stateless behaviour) but can be stateful if needed. The API spec should let clear that format retuns may be different when different video inputs (or outputs) are selected. === Note that this will cause ABI breakage with existing applications. If an application expects an error condition to become aware that the requested format was not supported, that application will silently think the requested format was valid but in fact the driver is returning data in some other format. Tvtime (one of the more popular apps for watching analog television) is one such application that will broken. http://git.linuxtv.org/tvtime.git/blob/HEAD:/src/videoinput.c#l452 If YUVY isn't supported but UYVY is (for example, with the Hauppauge HVR-950q), the application will think it's doing YUYV when in fact the driver is returning UYVY. MythTV is a little better (it does ultimately store the format returned by the driver), however even there it depends on an error being returned in order to know to do userland format conversion. https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythtv/recorders/NuppelVideoRecorder.cpp#L1367 Now it would be quite simple to change tvtime to use the expected behavior, but if backward compatibility with the ABI is of paramount importance, then we cannot proceed with this change as proposed. Don't misunderstand me - if I were inventing the API today then the proposed approach is what I would recommend - but since these parts of the ABI are something like ten years old, we have to take into account legacy applications. Discussion is welcome. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
