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 of them have side-effects, ie > v4lconvert_convert_pixfmt() calling v4lprocessing_processing(), which is > fine as is, but may be a problem when making it public. Yes, I have to think about that. > > I agree that the libv4lconvert API could use an overhaul, but one of the > first things to do would be to untangle the rather glued on atm > libv4lcontrol and libv4lprocessing bits. I have a (partial) plan for > this, > but no time. One of the first things which I would like to do is to > move over the software processing bits to a proper plugin model, where > by the plugins themselves also add the necessary "fake" v4l2-ctrls for > their own piece of functionality and where by the giant > quirk list in libv4lconvert/control/libv4lcontrol.c becomes split > in a quirk list per processing plugin, enabling that plugin for the > cameras which need that plugin. libv4lcontrol.c would then just > offer a generic mechanism to add fake controls, and to check if > a camera matches *a* quirklist, which the plugins can then use. Btw, did you ever think about making the device list a text file ? You know editing a text file is much easer than updating the whole library. ;) > > This would then allow for much easier addition of new software processing > plugins, such as for example software autofocus which is needed for > some cams. > > All of this can be done without any external API changes atm, and would > have real tangible results, esp. once we start adding more software > processing plugins. Where as exporting more of the libv4lcontrol > internals, > will make it much harder to do this much needed overhaul, and buys > us very little tangible advantages (again no normal end user applications > are using libv4lconvert directly). I agree. First overhaul the internals, then extend the API. > > So what I would like to see happen, if you want to work on > libv4lconvert is > to start with doing the much needed overhaul of the internals, and > once that > is done making a more generic API available is fine, and if we break > the API > in the process that is fine too. But lets start with doing the ground > work > on not with exporting internals which are not suitable for exporting atm. 100% agreement. > > Note that if there were a real need for exporting these internals > right now, > things would be different, but I don't see such a real need, and no, > sorry, > from my pov qv4l2 does not count, esp. since things can easily be fixed > there. > > Note that I've written a proposal for a libv4lprocessing plugins a > long time > ago, you can find it here: > http://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg18993.html Thanks, I will consider it. Regards, Frank > > Regards, > > Hans > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
