Re: qv4l2-bug / libv4lconvert API issue
Hi Hans, Am 30.09.2012 11:54, schrieb Hans de Goede: Hi, On 09/28/2012 07:09 PM, Frank Schäfer wrote: Hi, Am 27.09.2012 21:41, schrieb Hans de Goede: Hi, On 09/27/2012 03:20 PM, Frank Schäfer wrote: snip What you've found is a qv4l2 bug (do you have the latest version?) Of course, I'm using the latest developer version. Even if this is just a qv4l2-bug: how do you want to fix it without removing the format selction feature ? Well, if qv4l2 can only handle rgb24 data, then it should gray out the format selection (fixing it at rgb24) when not in raw mode. So you say just remove this feature from qv4l2. I prefer fixing the library / API instead. No I'm suggesting to keep the feature to select which input format to use when in raw mode, while at the same time disabling the feature) when in libv4l2 mode. What use is it to ask libv4l2 for say YUV420 data and then later ask libv4lconvert to convert this to RGB24, when you could have asked libv4l2 for RGB24 right away. I assume the idea behind input format selction when using libv4l2 is to provide a possibilty to test libv4l2 ? I'm not sure if we really need this. If not, sure, we can simply remove it to get rid of the problem. snip [we both agree that the current design/behavior is suboptimal, needs to be documented and should be improved] ... But saying that libv4l2 and libv4lconvert can't be used at the same (although they are separate public libraries) and that v4lconvert_convert() may only be called once per frame seems to me like a - I would say weird - reinterpretation of the API... I don't think this is what applications currently expect (qv4l2 doesn't ;) ) and at least this would need public clarification. Using the 2 at the same time never was the intention libv4lconvert lies *beneath* libv4l2 in the stack. Yeah, sure. Using them both at the same time would be like using some high level file io API, while at the same making lowlevel seek / read / write calls on the underlying fd and then expecting things to behave consistently. 00.9% of all apps should (and do) use the highlevel libv4l2 API. Some testing / developer apps like qv4l2 use libv4lconvert directly. The problem here is, that we actually have TWO high level APIs which interact in a - let's call it unfortunate - way. I disagree that they are both highlevel. I know of only 2 tools which use libv4lconvert directly, qv4l2 and svv, and both of them were written by kernel developers for driver testing. So in practice everyone who want a high level API is using libv4l2 (which is already low-level enough by it self). Hmmm... that's a weird argumentation. Because only a few applications are using it, it is not a high level API ? And assuming that we really know all users of a public API is very dangerous... This interaction is not clear for the users (due to the transparent processing stuff) and it doesn't match not what users expect. But I think we can fix it and gain some nice extra features as a bonus. Then lets document the interaction. I think you don't realize which can of worms you're opening when you try to make libv4lconvert more generally usable. Please read the v4lconvert_convert function very carefully, esp. the part where it hooks into v4lprocessing (software whitebalance, and gamma correction). Make sure you understand what v4lprocessing_pre_processing() does, what the difference is between convert = 1 and convert = 2, and where *all* the callers are of v4lprocessing_processing() (hint one is hiding in v4lconvert_convert_pixfmt) Yeah, it's a bit tricky, but understandable. At the moment, the plan is not to change WHAT is done in this function, but WHERE it is done. Make sure you understand every single line of v4lconvert_convert! Once you've done that I will welcome a proposal to make the libv4lconvert API more general usable which: 1) Does not break any existing use-cases 2) Does not slow down things in anyway (so which does not introduce any extra intermediate buffers) Sure. Will do that. Basically, the idea is to make libv4lconvert a pure toolbox with methods for - for V4L_PIX_FORMAT conversion - cropping - horizontal+vertical flipping - rotation - processing (applying filters = auto whitebalance, auto gain, gamma correction etc.) - ... of frames. Most of these methods are already there and only need to be made public (some with small extenstions/modifiactions). The whole magic (transparent flipping, rotation, ...) should be done inside libv4l. Does that make sense for you ? snip 2) What is the use case for having separate convert_pixfmt etc. functions available ... ? We already have them as separate functions, so the only question is: does it make sense to make them public ? I would say: yes, because this seems to be a sane way to fix a problem. And the second reason would be, that the provided operations are usefull for applications. My point is that currently some
Re: qv4l2-bug / libv4lconvert API issue
On Wed 3 October 2012 12:22:48 Frank Schäfer wrote: Hi Hans, Am 30.09.2012 11:54, schrieb Hans de Goede: Hi, On 09/28/2012 07:09 PM, Frank Schäfer wrote: Hi, Am 27.09.2012 21:41, schrieb Hans de Goede: Hi, On 09/27/2012 03:20 PM, Frank Schäfer wrote: snip What you've found is a qv4l2 bug (do you have the latest version?) Of course, I'm using the latest developer version. Even if this is just a qv4l2-bug: how do you want to fix it without removing the format selction feature ? Well, if qv4l2 can only handle rgb24 data, then it should gray out the format selection (fixing it at rgb24) when not in raw mode. So you say just remove this feature from qv4l2. I prefer fixing the library / API instead. No I'm suggesting to keep the feature to select which input format to use when in raw mode, while at the same time disabling the feature) when in libv4l2 mode. What use is it to ask libv4l2 for say YUV420 data and then later ask libv4lconvert to convert this to RGB24, when you could have asked libv4l2 for RGB24 right away. I assume the idea behind input format selction when using libv4l2 is to provide a possibilty to test libv4l2 ? The main reason why I show all formats is that the driver reports all these formats, so one should be able to select them in order to test the driver. And I'm using libv4l2convert so that I can actually see a picture. For formats like MPEG that are unsupported by libv4l2convert I just dump the 'image' as is. It is counterintuitive if a YUV format is converted to a proper picture using qv4l2 -r, but that it is all wrong with qv4l2. I'm all for improving the library. 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: qv4l2-bug / libv4lconvert API issue
Am 03.10.2012 15:32, schrieb Hans Verkuil: On Wed 3 October 2012 12:22:48 Frank Schäfer wrote: Hi Hans, Am 30.09.2012 11:54, schrieb Hans de Goede: Hi, On 09/28/2012 07:09 PM, Frank Schäfer wrote: Hi, Am 27.09.2012 21:41, schrieb Hans de Goede: Hi, On 09/27/2012 03:20 PM, Frank Schäfer wrote: snip What you've found is a qv4l2 bug (do you have the latest version?) Of course, I'm using the latest developer version. Even if this is just a qv4l2-bug: how do you want to fix it without removing the format selction feature ? Well, if qv4l2 can only handle rgb24 data, then it should gray out the format selection (fixing it at rgb24) when not in raw mode. So you say just remove this feature from qv4l2. I prefer fixing the library / API instead. No I'm suggesting to keep the feature to select which input format to use when in raw mode, while at the same time disabling the feature) when in libv4l2 mode. What use is it to ask libv4l2 for say YUV420 data and then later ask libv4lconvert to convert this to RGB24, when you could have asked libv4l2 for RGB24 right away. I assume the idea behind input format selction when using libv4l2 is to provide a possibilty to test libv4l2 ? The main reason why I show all formats is that the driver reports all these formats, so one should be able to select them in order to test the driver. And I'm using libv4l2convert so that I can actually see a picture. For formats like MPEG that are unsupported by libv4l2convert I just dump the 'image' as is. Yes, but for pure testing of the driver output formats, it is better to open the device in raw mode and convert the picture for GUI-output with a v4lconvert_convert() call. It is counterintuitive if a YUV format is converted to a proper picture using qv4l2 -r, but that it is all wrong with qv4l2. I'm not sure I understand what you mean... Regards, Frank I'm all for improving the library. 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: qv4l2-bug / libv4lconvert API issue
Hi, On 09/28/2012 07:09 PM, Frank Schäfer wrote: Hi, Am 27.09.2012 21:41, schrieb Hans de Goede: Hi, On 09/27/2012 03:20 PM, Frank Schäfer wrote: snip What you've found is a qv4l2 bug (do you have the latest version?) Of course, I'm using the latest developer version. Even if this is just a qv4l2-bug: how do you want to fix it without removing the format selction feature ? Well, if qv4l2 can only handle rgb24 data, then it should gray out the format selection (fixing it at rgb24) when not in raw mode. So you say just remove this feature from qv4l2. I prefer fixing the library / API instead. No I'm suggesting to keep the feature to select which input format to use when in raw mode, while at the same time disabling the feature) when in libv4l2 mode. What use is it to ask libv4l2 for say YUV420 data and then later ask libv4lconvert to convert this to RGB24, when you could have asked libv4l2 for RGB24 right away. v4lconvert_convert should only be called with src_fmt and dest_fmt parameters which are the result of a v4lconvert_try_format call, which clearly is not the case here! Why ? Because our library code is broken ? Because that is a pre-condition which v4lconvert_convert expects to be met. Not meeting that pre-condition means operating outside of the designated operating parameters, and as such weird things may happen. Is this important restriction documented somewhere ? Not explicitly, patches welcome. one is supposed to either use libv4l2, or do raw device access and then call libv4lconvert directly. Even when using libv4lconvert only, multiple frame conversions are still causing the same trouble. True, but doing multiple conversions on one frame is just crazy ... Well, I would say not necessary in most cases. But I can certainly think of use cases (other than what qv4l2 does). At least it should be possible and I think this is what application programmers expect when using a conversion function from a library. Right, as said before libv4lconvert is not meant as a generic format conversion library, and using it as such is not necessarily a good idea as there are much better alternatives out there for doing generic format conversion (both more flexible and faster). More over libv4lconvert is specifically designed to be called once per frame. Yes another restriction that needs documenting. But saying that libv4l2 and libv4lconvert can't be used at the same (although they are separate public libraries) and that v4lconvert_convert() may only be called once per frame seems to me like a - I would say weird - reinterpretation of the API... I don't think this is what applications currently expect (qv4l2 doesn't ;) ) and at least this would need public clarification. Using the 2 at the same time never was the intention libv4lconvert lies *beneath* libv4l2 in the stack. Yeah, sure. Using them both at the same time would be like using some high level file io API, while at the same making lowlevel seek / read / write calls on the underlying fd and then expecting things to behave consistently. 00.9% of all apps should (and do) use the highlevel libv4l2 API. Some testing / developer apps like qv4l2 use libv4lconvert directly. The problem here is, that we actually have TWO high level APIs which interact in a - let's call it unfortunate - way. I disagree that they are both highlevel. I know of only 2 tools which use libv4lconvert directly, qv4l2 and svv, and both of them were written by kernel developers for driver testing. So in practice everyone who want a high level API is using libv4l2 (which is already low-level enough by it self). This interaction is not clear for the users (due to the transparent processing stuff) and it doesn't match not what users expect. But I think we can fix it and gain some nice extra features as a bonus. Then lets document the interaction. I think you don't realize which can of worms you're opening when you try to make libv4lconvert more generally usable. Please read the v4lconvert_convert function very carefully, esp. the part where it hooks into v4lprocessing (software whitebalance, and gamma correction). Make sure you understand what v4lprocessing_pre_processing() does, what the difference is between convert = 1 and convert = 2, and where *all* the callers are of v4lprocessing_processing() (hint one is hiding in v4lconvert_convert_pixfmt) Make sure you understand every single line of v4lconvert_convert! Once you've done that I will welcome a proposal to make the libv4lconvert API more general usable which: 1) Does not break any existing use-cases 2) Does not slow down things in anyway (so which does not introduce any extra intermediate buffers) snip 2) What is the use case for having separate convert_pixfmt etc. functions available ... ? We already have them as separate functions, so the only question is: does it make sense to make them public ? I would say: yes, because this seems to be a sane way to fix a
Re: qv4l2-bug / libv4lconvert API issue
Hi, Am 27.09.2012 21:41, schrieb Hans de Goede: Hi, On 09/27/2012 03:20 PM, Frank Schäfer wrote: snip What you've found is a qv4l2 bug (do you have the latest version?) Of course, I'm using the latest developer version. Even if this is just a qv4l2-bug: how do you want to fix it without removing the format selction feature ? Well, if qv4l2 can only handle rgb24 data, then it should gray out the format selection (fixing it at rgb24) when not in raw mode. So you say just remove this feature from qv4l2. I prefer fixing the library / API instead. v4lconvert_convert should only be called with src_fmt and dest_fmt parameters which are the result of a v4lconvert_try_format call, which clearly is not the case here! Why ? Because our library code is broken ? ;) Is this important restriction documented somewhere ? one is supposed to either use libv4l2, or do raw device access and then call libv4lconvert directly. Even when using libv4lconvert only, multiple frame conversions are still causing the same trouble. True, but doing multiple conversions on one frame is just crazy ... Well, I would say not necessary in most cases. But I can certainly think of use cases (other than what qv4l2 does). At least it should be possible and I think this is what application programmers expect when using a conversion function from a library. Hans, first of all, I would like to say that my intention is not to savage your work. I know API design and maintanance is very difficult and you are doing a great job. Things like this just happen and we have to make the best out of it. I will gladly admit that since libv4lconvert has organically grown things like flipping and software processing, the API is no longer ideal :) So let's improve it ! :) But saying that libv4l2 and libv4lconvert can't be used at the same (although they are separate public libraries) and that v4lconvert_convert() may only be called once per frame seems to me like a - I would say weird - reinterpretation of the API... I don't think this is what applications currently expect (qv4l2 doesn't ;) ) and at least this would need public clarification. Using the 2 at the same time never was the intention libv4lconvert lies *beneath* libv4l2 in the stack. Yeah, sure. Using them both at the same time would be like using some high level file io API, while at the same making lowlevel seek / read / write calls on the underlying fd and then expecting things to behave consistently. 00.9% of all apps should (and do) use the highlevel libv4l2 API. Some testing / developer apps like qv4l2 use libv4lconvert directly. The problem here is, that we actually have TWO high level APIs which interact in a - let's call it unfortunate - way. This interaction is not clear for the users (due to the transparent processing stuff) and it doesn't match not what users expect. But I think we can fix it and gain some nice extra features as a bonus. And I must admit that I've considered simple making the libv4lconvert API private at times. :D That would certainly make things consistent ;) So let's see if there is a possibility to fix the issue by improving the libraries without breaking the API. What about the following solution: - make v4lconvert_convert_pixfmt() and v4lconvert_crop() public functions and fix qv4l2 to use them instead of v4lconvert_convert() - also make the flip and rotation functions (v4lconvert_flip(), v4lconvert_rotate90()) publicly available That would need some careful review of their API's but after that yes that should be doable. - modify libv4l2 to call v4lconvert_convert_pixfmt() and the flip+rotation+crop functions instead of v4lconvert_convert() - fix v4lconvert_convert() to not do transparent image flipping/rotation (means = call v4lconvert_convert_pixfmt() and v4lconvert_crop() only). Erm, no, have you looked at v4lconvert_convert it does a lot of magic with figuring out how much intermediate buffers it needs (skipping steps where possible), caches those buffers rather then re-alloc them every frame, etc. Making it do less means loosing some chances for optimization, and frankly it works well. I don't see why we would need some do some stuff but not all function when we also offer all the separate steps for users who want them. Did you notice the mail I've sent a few minutes later ? ;) I agree, we have to keep it as is but should mark it as deprecated. Also I still consider the rotate 90 for pac7302 part of the pixfmt conversion, this is something specific to how these cameras encode the picture, not a generic thing. Yes, but why not make it a generic feature ? Would be nice to have. (ever had a webcam with a clamp socket ?) But this is just about a side effect, lets concentrate on the main issue. For API-clean-up: - create a copy of (the fixed) v4lconvert_convert() called something like v4lconvert_convert_crop() - declare v4lconvert_convert() as
Re: qv4l2-bug / libv4lconvert API issue
Hi, On 09/26/2012 11:04 PM, Frank Schäfer wrote: Hi, I've noticed the following issues/bugs while playing with qv4l: 1.) with pac7302-webcams, only the RGB3 (RGB24) format is working. BGR3, YU12 and YV12 are broken 2.) for upside-down-mounted devices with an entry in libv4lconvert, automatic h/v-flipping doesn't work with some formats I've been digging a bit deeper into the code and it seems that both issues are caused by a problem with the libv4lconvert-API: Besides image format conversion, function v4lconvert_convert() also does the automatic image flipping and rotation (for devices with flags V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED and V4LCONTROL_ROTATED_90_JPEG) The problem is, that this function can be called multiple times for the same frame, which then of course results in repeated flipping and rotation... And this is exactly what happens with qv4l2: qv4l2 gets the frame from libv4l2, which calls v4lconvert_convert() in v4l2_dequeue_and_convert() or v4l2_read_and_convert(). The retrieved frame has the requested format and is already flipped/rotated. qv4l2 then calls v4lconvert_convert() again directly to convert the frame to RGB24 for GUI-output and this is where things are going wrong. In case of h/v-flip, the double conversion only equalizes the V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED flags, but for rotated devices, the image gets corrupted. Sure, what qv4l2 does is a crazy. Applications usually request the format needed for GUI-output directly from libv4l2. Anyway, as long as it is valid to call libv4lconvert directly, we can not assume that v4lconvert_convert() is called only one time. At the moment, I see no possibility to fix this without changing the libv4lconvert-API. Thoughts ? What you've found is a qv4l2 bug (do you have the latest version?) one is supposed to either use libv4l2, or do raw device access and then call libv4lconvert directly. These are also the 2 modes qv4l2 has (for testing purposes), it is not supposed to do the manual convert call when using libv4l2 to access the device ... 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: qv4l2-bug / libv4lconvert API issue
Hi, Am 27.09.2012 12:26, schrieb Hans de Goede: Hi, On 09/26/2012 11:04 PM, Frank Schäfer wrote: Hi, I've noticed the following issues/bugs while playing with qv4l: 1.) with pac7302-webcams, only the RGB3 (RGB24) format is working. BGR3, YU12 and YV12 are broken 2.) for upside-down-mounted devices with an entry in libv4lconvert, automatic h/v-flipping doesn't work with some formats I've been digging a bit deeper into the code and it seems that both issues are caused by a problem with the libv4lconvert-API: Besides image format conversion, function v4lconvert_convert() also does the automatic image flipping and rotation (for devices with flags V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED and V4LCONTROL_ROTATED_90_JPEG) The problem is, that this function can be called multiple times for the same frame, which then of course results in repeated flipping and rotation... And this is exactly what happens with qv4l2: qv4l2 gets the frame from libv4l2, which calls v4lconvert_convert() in v4l2_dequeue_and_convert() or v4l2_read_and_convert(). The retrieved frame has the requested format and is already flipped/rotated. qv4l2 then calls v4lconvert_convert() again directly to convert the frame to RGB24 for GUI-output and this is where things are going wrong. In case of h/v-flip, the double conversion only equalizes the V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED flags, but for rotated devices, the image gets corrupted. Sure, what qv4l2 does is a crazy. Applications usually request the format needed for GUI-output directly from libv4l2. Anyway, as long as it is valid to call libv4lconvert directly, we can not assume that v4lconvert_convert() is called only one time. At the moment, I see no possibility to fix this without changing the libv4lconvert-API. Thoughts ? What you've found is a qv4l2 bug (do you have the latest version?) Of course, I'm using the latest developer version. Even if this is just a qv4l2-bug: how do you want to fix it without removing the format selction feature ? one is supposed to either use libv4l2, or do raw device access and then call libv4lconvert directly. Even when using libv4lconvert only, multiple frame conversions are still causing the same trouble. Hans, first of all, I would like to say that my intention is not to savage your work. I know API design and maintanance is very difficult and you are doing a great job. Things like this just happen and we have to make the best out of it. But saying that libv4l2 and libv4lconvert can't be used at the same (although they are separate public libraries) and that v4lconvert_convert() may only be called once per frame seems to me like a - I would say weird - reinterpretation of the API... I don't think this is what applications currently expect (qv4l2 doesn't ;) ) and at least this would need public clarification. So let's see if there is a possibility to fix the issue by improving the libraries without breaking the API. What about the following solution: - make v4lconvert_convert_pixfmt() and v4lconvert_crop() public functions and fix qv4l2 to use them instead of v4lconvert_convert() - also make the flip and rotation functions (v4lconvert_flip(), v4lconvert_rotate90()) publicly available - modify libv4l2 to call v4lconvert_convert_pixfmt() and the flip+rotation+crop functions instead of v4lconvert_convert() - fix v4lconvert_convert() to not do transparent image flipping/rotation (means = call v4lconvert_convert_pixfmt() and v4lconvert_crop() only). For API-clean-up: - create a copy of (the fixed) v4lconvert_convert() called something like v4lconvert_convert_crop() - declare v4lconvert_convert() as deprecated so that we can remove it sometime in the future What do you think ? Regards, Frank These are also the 2 modes qv4l2 has (for testing purposes), it is not supposed to do the manual convert call when using libv4l2 to access the device ... 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: qv4l2-bug / libv4lconvert API issue
Am 27.09.2012 15:20, schrieb Frank Schäfer: Hi, Am 27.09.2012 12:26, schrieb Hans de Goede: Hi, On 09/26/2012 11:04 PM, Frank Schäfer wrote: Hi, I've noticed the following issues/bugs while playing with qv4l: 1.) with pac7302-webcams, only the RGB3 (RGB24) format is working. BGR3, YU12 and YV12 are broken 2.) for upside-down-mounted devices with an entry in libv4lconvert, automatic h/v-flipping doesn't work with some formats I've been digging a bit deeper into the code and it seems that both issues are caused by a problem with the libv4lconvert-API: Besides image format conversion, function v4lconvert_convert() also does the automatic image flipping and rotation (for devices with flags V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED and V4LCONTROL_ROTATED_90_JPEG) The problem is, that this function can be called multiple times for the same frame, which then of course results in repeated flipping and rotation... And this is exactly what happens with qv4l2: qv4l2 gets the frame from libv4l2, which calls v4lconvert_convert() in v4l2_dequeue_and_convert() or v4l2_read_and_convert(). The retrieved frame has the requested format and is already flipped/rotated. qv4l2 then calls v4lconvert_convert() again directly to convert the frame to RGB24 for GUI-output and this is where things are going wrong. In case of h/v-flip, the double conversion only equalizes the V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED flags, but for rotated devices, the image gets corrupted. Sure, what qv4l2 does is a crazy. Applications usually request the format needed for GUI-output directly from libv4l2. Anyway, as long as it is valid to call libv4lconvert directly, we can not assume that v4lconvert_convert() is called only one time. At the moment, I see no possibility to fix this without changing the libv4lconvert-API. Thoughts ? What you've found is a qv4l2 bug (do you have the latest version?) Of course, I'm using the latest developer version. Even if this is just a qv4l2-bug: how do you want to fix it without removing the format selction feature ? one is supposed to either use libv4l2, or do raw device access and then call libv4lconvert directly. Even when using libv4lconvert only, multiple frame conversions are still causing the same trouble. Hans, first of all, I would like to say that my intention is not to savage your work. I know API design and maintanance is very difficult and you are doing a great job. Things like this just happen and we have to make the best out of it. But saying that libv4l2 and libv4lconvert can't be used at the same (although they are separate public libraries) and that v4lconvert_convert() may only be called once per frame seems to me like a - I would say weird - reinterpretation of the API... I don't think this is what applications currently expect (qv4l2 doesn't ;) ) and at least this would need public clarification. So let's see if there is a possibility to fix the issue by improving the libraries without breaking the API. What about the following solution: - make v4lconvert_convert_pixfmt() and v4lconvert_crop() public functions and fix qv4l2 to use them instead of v4lconvert_convert() - also make the flip and rotation functions (v4lconvert_flip(), v4lconvert_rotate90()) publicly available - modify libv4l2 to call v4lconvert_convert_pixfmt() and the flip+rotation+crop functions instead of v4lconvert_convert() - fix v4lconvert_convert() to not do transparent image flipping/rotation (means = call v4lconvert_convert_pixfmt() and v4lconvert_crop() only). No, of course we can't do that... So let's leave v4lconvert_convert() as broken as it is and just mark it as deprecated. The rest should work. Frank For API-clean-up: - create a copy of (the fixed) v4lconvert_convert() called something like v4lconvert_convert_crop() - declare v4lconvert_convert() as deprecated so that we can remove it sometime in the future What do you think ? Regards, Frank These are also the 2 modes qv4l2 has (for testing purposes), it is not supposed to do the manual convert call when using libv4l2 to access the device ... 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: qv4l2-bug / libv4lconvert API issue
On Thu 27 September 2012 15:20:21 Frank Schäfer wrote: Hi, Am 27.09.2012 12:26, schrieb Hans de Goede: Hi, On 09/26/2012 11:04 PM, Frank Schäfer wrote: Hi, I've noticed the following issues/bugs while playing with qv4l: 1.) with pac7302-webcams, only the RGB3 (RGB24) format is working. BGR3, YU12 and YV12 are broken 2.) for upside-down-mounted devices with an entry in libv4lconvert, automatic h/v-flipping doesn't work with some formats I've been digging a bit deeper into the code and it seems that both issues are caused by a problem with the libv4lconvert-API: Besides image format conversion, function v4lconvert_convert() also does the automatic image flipping and rotation (for devices with flags V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED and V4LCONTROL_ROTATED_90_JPEG) The problem is, that this function can be called multiple times for the same frame, which then of course results in repeated flipping and rotation... And this is exactly what happens with qv4l2: qv4l2 gets the frame from libv4l2, which calls v4lconvert_convert() in v4l2_dequeue_and_convert() or v4l2_read_and_convert(). The retrieved frame has the requested format and is already flipped/rotated. qv4l2 then calls v4lconvert_convert() again directly to convert the frame to RGB24 for GUI-output and this is where things are going wrong. qv4l2 will do things the way you expect if you open the video node in 'raw' mode (i.e. without using libv4l2), either through the File menu or by adding the -r option: qv4l2 -r /dev/video0 BTW, which pixelformat do you select? I think that if you select RGB24, then no extra conversion step will take place. Note that qv4l2 is primarily meant for testing. Normal applications will never call v4lconvert_convert(). That said, qv4l2 has the potential of becoming a much more usable application if someone would be interested in putting some more work into it. In case of h/v-flip, the double conversion only equalizes the V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED flags, but for rotated devices, the image gets corrupted. Sure, what qv4l2 does is a crazy. Applications usually request the format needed for GUI-output directly from libv4l2. Anyway, as long as it is valid to call libv4lconvert directly, we can not assume that v4lconvert_convert() is called only one time. At the moment, I see no possibility to fix this without changing the libv4lconvert-API. Thoughts ? What you've found is a qv4l2 bug (do you have the latest version?) Of course, I'm using the latest developer version. Even if this is just a qv4l2-bug: how do you want to fix it without removing the format selction feature ? one is supposed to either use libv4l2, or do raw device access and then call libv4lconvert directly. Even when using libv4lconvert only, multiple frame conversions are still causing the same trouble. Hans, first of all, I would like to say that my intention is not to savage your work. I know API design and maintanance is very difficult and you are doing a great job. Things like this just happen and we have to make the best out of it. But saying that libv4l2 and libv4lconvert can't be used at the same (although they are separate public libraries) and that v4lconvert_convert() may only be called once per frame seems to me like a - I would say weird - reinterpretation of the API... I don't think this is what applications currently expect (qv4l2 doesn't ;) ) and at least this would need public clarification. So let's see if there is a possibility to fix the issue by improving the libraries without breaking the API. What about the following solution: - make v4lconvert_convert_pixfmt() and v4lconvert_crop() public functions and fix qv4l2 to use them instead of v4lconvert_convert() - also make the flip and rotation functions (v4lconvert_flip(), v4lconvert_rotate90()) publicly available - modify libv4l2 to call v4lconvert_convert_pixfmt() and the flip+rotation+crop functions instead of v4lconvert_convert() - fix v4lconvert_convert() to not do transparent image flipping/rotation (means = call v4lconvert_convert_pixfmt() and v4lconvert_crop() only). For API-clean-up: - create a copy of (the fixed) v4lconvert_convert() called something like v4lconvert_convert_crop() - declare v4lconvert_convert() as deprecated so that we can remove it sometime in the future As qv4l2 maintainer I would definitely be interested in seeing this happen. On the other hand, qv4l2 is an unusual application. Normal apps are unlikely to use this feature, although I can imagine it being used in some embedded environments. Regards, Hans V. What do you think ? Regards, Frank These are also the 2 modes qv4l2 has (for testing purposes), it is not supposed to do the manual convert call when using libv4l2 to access the device ... Regards, Hans -- To unsubscribe from this list: send the line unsubscribe
Re: qv4l2-bug / libv4lconvert API issue
Am 27.09.2012 16:47, schrieb Hans Verkuil: On Thu 27 September 2012 15:20:21 Frank Schäfer wrote: Hi, Am 27.09.2012 12:26, schrieb Hans de Goede: Hi, On 09/26/2012 11:04 PM, Frank Schäfer wrote: Hi, I've noticed the following issues/bugs while playing with qv4l: 1.) with pac7302-webcams, only the RGB3 (RGB24) format is working. BGR3, YU12 and YV12 are broken 2.) for upside-down-mounted devices with an entry in libv4lconvert, automatic h/v-flipping doesn't work with some formats I've been digging a bit deeper into the code and it seems that both issues are caused by a problem with the libv4lconvert-API: Besides image format conversion, function v4lconvert_convert() also does the automatic image flipping and rotation (for devices with flags V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED and V4LCONTROL_ROTATED_90_JPEG) The problem is, that this function can be called multiple times for the same frame, which then of course results in repeated flipping and rotation... And this is exactly what happens with qv4l2: qv4l2 gets the frame from libv4l2, which calls v4lconvert_convert() in v4l2_dequeue_and_convert() or v4l2_read_and_convert(). The retrieved frame has the requested format and is already flipped/rotated. qv4l2 then calls v4lconvert_convert() again directly to convert the frame to RGB24 for GUI-output and this is where things are going wrong. qv4l2 will do things the way you expect if you open the video node in 'raw' mode (i.e. without using libv4l2), either through the File menu or by adding the -r option: qv4l2 -r /dev/video0 Yes, in raw mode it should work (didn't test). BTW, which pixelformat do you select? I think that if you select RGB24, then no extra conversion step will take place. That's what I said. Note that qv4l2 is primarily meant for testing. Normal applications will never call v4lconvert_convert(). I remember having seen one, but I can't remember details :( Last application I've sent patches for was Kopete, but that one only uses libv4l2 (don't look into its video code, it still makes you blind !) :( Anyway, it's a public API and therefore we have to assume that applications are using it. ;) That said, qv4l2 has the potential of becoming a much more usable application if someone would be interested in putting some more work into it. I agree, it has potential. And the current version has gained some nice features ! Maybe I will send some patches in the future. In case of h/v-flip, the double conversion only equalizes the V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED flags, but for rotated devices, the image gets corrupted. Sure, what qv4l2 does is a crazy. Applications usually request the format needed for GUI-output directly from libv4l2. Anyway, as long as it is valid to call libv4lconvert directly, we can not assume that v4lconvert_convert() is called only one time. At the moment, I see no possibility to fix this without changing the libv4lconvert-API. Thoughts ? What you've found is a qv4l2 bug (do you have the latest version?) Of course, I'm using the latest developer version. Even if this is just a qv4l2-bug: how do you want to fix it without removing the format selction feature ? one is supposed to either use libv4l2, or do raw device access and then call libv4lconvert directly. Even when using libv4lconvert only, multiple frame conversions are still causing the same trouble. Hans, first of all, I would like to say that my intention is not to savage your work. I know API design and maintanance is very difficult and you are doing a great job. Things like this just happen and we have to make the best out of it. But saying that libv4l2 and libv4lconvert can't be used at the same (although they are separate public libraries) and that v4lconvert_convert() may only be called once per frame seems to me like a - I would say weird - reinterpretation of the API... I don't think this is what applications currently expect (qv4l2 doesn't ;) ) and at least this would need public clarification. So let's see if there is a possibility to fix the issue by improving the libraries without breaking the API. What about the following solution: - make v4lconvert_convert_pixfmt() and v4lconvert_crop() public functions and fix qv4l2 to use them instead of v4lconvert_convert() - also make the flip and rotation functions (v4lconvert_flip(), v4lconvert_rotate90()) publicly available - modify libv4l2 to call v4lconvert_convert_pixfmt() and the flip+rotation+crop functions instead of v4lconvert_convert() - fix v4lconvert_convert() to not do transparent image flipping/rotation (means = call v4lconvert_convert_pixfmt() and v4lconvert_crop() only). For API-clean-up: - create a copy of (the fixed) v4lconvert_convert() called something like v4lconvert_convert_crop() - declare v4lconvert_convert() as deprecated so that we can remove it sometime in the future As qv4l2 maintainer Sorry, I didn't notice that. I thought Hans (de
Re: qv4l2-bug / libv4lconvert API issue
Hi, On 09/27/2012 03:20 PM, Frank Schäfer wrote: snip What you've found is a qv4l2 bug (do you have the latest version?) Of course, I'm using the latest developer version. Even if this is just a qv4l2-bug: how do you want to fix it without removing the format selction feature ? Well, if qv4l2 can only handle rgb24 data, then it should gray out the format selection (fixing it at rgb24) when not in raw mode. v4lconvert_convert should only be called with src_fmt and dest_fmt parameters which are the result of a v4lconvert_try_format call, which clearly is not the case here! one is supposed to either use libv4l2, or do raw device access and then call libv4lconvert directly. Even when using libv4lconvert only, multiple frame conversions are still causing the same trouble. True, but doing multiple conversions on one frame is just crazy ... Hans, first of all, I would like to say that my intention is not to savage your work. I know API design and maintanance is very difficult and you are doing a great job. Things like this just happen and we have to make the best out of it. I will gladly admit that since libv4lconvert has organically grown things like flipping and software processing, the API is no longer ideal :) But saying that libv4l2 and libv4lconvert can't be used at the same (although they are separate public libraries) and that v4lconvert_convert() may only be called once per frame seems to me like a - I would say weird - reinterpretation of the API... I don't think this is what applications currently expect (qv4l2 doesn't ;) ) and at least this would need public clarification. Using the 2 at the same time never was the intention libv4lconvert lies *beneath* libv4l2 in the stack. Using them both at the same time would be like using some high level file io API, while at the same making lowlevel seek / read / write calls on the underlying fd and then expecting things to behave consistently. 00.9% of all apps should (and do) use the highlevel libv4l2 API. Some testing / developer apps like qv4l2 use libv4lconvert directly. And I must admit that I've considered simple making the libv4lconvert API private at times. So let's see if there is a possibility to fix the issue by improving the libraries without breaking the API. What about the following solution: - make v4lconvert_convert_pixfmt() and v4lconvert_crop() public functions and fix qv4l2 to use them instead of v4lconvert_convert() - also make the flip and rotation functions (v4lconvert_flip(), v4lconvert_rotate90()) publicly available That would need some careful review of their API's but after that yes that should be doable. - modify libv4l2 to call v4lconvert_convert_pixfmt() and the flip+rotation+crop functions instead of v4lconvert_convert() - fix v4lconvert_convert() to not do transparent image flipping/rotation (means = call v4lconvert_convert_pixfmt() and v4lconvert_crop() only). Erm, no, have you looked at v4lconvert_convert it does a lot of magic with figuring out how much intermediate buffers it needs (skipping steps where possible), caches those buffers rather then re-alloc them every frame, etc. Making it do less means loosing some chances for optimization, and frankly it works well. I don't see why we would need some do some stuff but not all function when we also offer all the separate steps for users who want them. Also I still consider the rotate 90 for pac7302 part of the pixfmt conversion, this is something specific to how these cameras encode the picture, not a generic thing. For API-clean-up: - create a copy of (the fixed) v4lconvert_convert() called something like v4lconvert_convert_crop() - declare v4lconvert_convert() as deprecated so that we can remove it sometime in the future What do you think ? 2 things: 1) qv4l2 needs to be fixed to not show to format selection in wrapped mode 2) What is the use case for having separate convert_pixfmt etc. functions available ... ? 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
qv4l2-bug / libv4lconvert API issue
Hi, I've noticed the following issues/bugs while playing with qv4l: 1.) with pac7302-webcams, only the RGB3 (RGB24) format is working. BGR3, YU12 and YV12 are broken 2.) for upside-down-mounted devices with an entry in libv4lconvert, automatic h/v-flipping doesn't work with some formats I've been digging a bit deeper into the code and it seems that both issues are caused by a problem with the libv4lconvert-API: Besides image format conversion, function v4lconvert_convert() also does the automatic image flipping and rotation (for devices with flags V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED and V4LCONTROL_ROTATED_90_JPEG) The problem is, that this function can be called multiple times for the same frame, which then of course results in repeated flipping and rotation... And this is exactly what happens with qv4l2: qv4l2 gets the frame from libv4l2, which calls v4lconvert_convert() in v4l2_dequeue_and_convert() or v4l2_read_and_convert(). The retrieved frame has the requested format and is already flipped/rotated. qv4l2 then calls v4lconvert_convert() again directly to convert the frame to RGB24 for GUI-output and this is where things are going wrong. In case of h/v-flip, the double conversion only equalizes the V4LCONTROL_HFLIPPED, V4LCONTROL_VFLIPPED flags, but for rotated devices, the image gets corrupted. Sure, what qv4l2 does is a crazy. Applications usually request the format needed for GUI-output directly from libv4l2. Anyway, as long as it is valid to call libv4lconvert directly, we can not assume that v4lconvert_convert() is called only one time. At the moment, I see no possibility to fix this without changing the libv4lconvert-API. Thoughts ? Regards, Frank -- 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
