Hi Sylwester,

On Sat, Nov 26, 2011 at 01:21:01PM +0100, Sylwester Nawrocki wrote:
...
> Did you have a chance to look at this RFC [1] ? I have reviewed usage 
> of VIDIOC_[S/G]_JPEGCOMP ioctls and there are also some links to previous 
> discussion there. 

I have to admit I had completely missed this RFC. I'll take a look at it.

> 
> > 
> > 2) Define a new control for jpeg quality. Its value range can be what the
> > hardware supports and the user space gets much better information on the
> > capabilities of the hardware and the granularity of the quality setting.
> 
> This was my conclusion as well. That sounds like a most effective and 
> flexible 
> solution. IIRC, Hans also suggested those things might be a perfect candidate
> for a control class.

Sounds like a good idea. Would the class be intended for all compressed
formats or just jpeg? I have to say I don't know much about the
alternatives, except that their compression ratio tends to be much better on
equivalent quality.

> 
> > 
> > I might even favour the second one. I also wonder how many user space
> > applications use this IOCTL, so if we're breaking anything by not supporting
> > it.
> 
> I imagine we could deprecate 'quality' and 'jpeg_markers' fields of 
> v4l2_jpegcompression
> and during deprecation period add support for the controls for these 
> parameters,
> so the application can adapt and fall back to a control based interface once 
> the
> ioctl is removed in the kernel.
> This should be possible for almost all drivers as they virtually use 
> G/S_JPEGCOMP 
> ioctls for image quality only.  
> 
> > 
> > Or we could decide to do option 1 right now and implement 2) later on. I can
> > write a patch to change the documentation.
> 
> The documentation could always be improved. But I personally would like to 
> see these
> ioctls die, as they're really hopeless as a part of public API. I though about
> something like
> 
> /* The ioctls to configure/query selected data segment in a jpeg encoder */  
> #define VIDIOC_G_JPEGCOMP        _IOR('V', 61, struct v4l2_jpegcomp)
> #define VIDIOC_S_JPEGCOMP        _IOW('V', 62, struct v4l2_jpegcomp)
> 
> /**
>  * struct v4l2_jpegcomp - JPEG segment data structure
>  *
>  * @id: field indicating what standard JPEG data segment @data contains
>  * @data: points the segment data
>  * @length: length of the segment
>  */
> struct v4l2_jpegcomp {
>       __u32 id;                
> 
>         int  length;
>         void  *data;
> };
> 
> 
> As a side note, our plan was to get merged the S5P JPEG codec in v3.3, with 
> the old jpeg ioctls and then switch to the controls when they're ready, 
> possibly
> in subsequent kernel release. That's why the driver is marked as experimental.

Sounds good to me.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi     jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
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

Reply via email to