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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
