RE: [RFC/PATCH v2] v4l: add control definitions for codec devices.

2011-06-03 Thread Jeongtae Park
Hi,

> -Original Message-
> From: Kamil Debski [mailto:k.deb...@samsung.com]
> Sent: Thursday, June 02, 2011 11:44 PM
> To: jtp.p...@samsung.com; linux-media@vger.kernel.org
> Cc: jaeryul...@samsung.com; june@samsung.com; janghyuck@samsung.com; 
> kyungmin.p...@samsung.com;
> younglak1004@samsung.com; 'Marek Szyprowski'
> Subject: RE: [RFC/PATCH v2] v4l: add control definitions for codec devices.
> 
> > From: Jeongtae Park [mailto:jtp.p...@samsung.com]
> > Sent: 02 June 2011 09:44
> > To: 'Kamil Debski'; linux-media@vger.kernel.org
> > Cc: jaeryul...@samsung.com; june@samsung.com; janghyuck@samsung.com;
> > kyungmin.p...@samsung.com; younglak1004....@samsung.com; 'Marek Szyprowski'
> > Subject: RE: [RFC/PATCH v2] v4l: add control definitions for codec devices.
> >
> > Hi,
> >
> > Thank you for your nice work. Here are my some comments.
> 
> Hi,
> 
> Thanks for your comments. I think I must have posted the wrong patch file...
> I will send the proper one, with some of your suggestion in a minute.
> 
> >
> > > -Original Message-
> > > From: Kamil Debski [mailto:k.deb...@samsung.com]
> > > Sent: Tuesday, May 31, 2011 6:08 PM
> > > Cc: m.szyprow...@samsung.com; kyungmin.p...@samsung.com;
> > k.deb...@samsung.com; jaeryul...@samsung.com;
> > > hverk...@xs4all.nl; laurent.pinch...@ideasonboard.com;
> > jtp.p...@samsung.com
> > > Subject: [RFC/PATCH v2] v4l: add control definitions for codec devices.
> > >
> > > Hi,
> > >
> > > This is a second version of the patch that adds controls for the codec
> > family of
> > > devices. I have implemented the suggestions I got from Hans Verkuil on the
> > #v4l
> > > channel.
> > >
> > > Change log:
> > > - rename V4L2_CID_MIN_REQ_BUFS_(CAP/OUT) to
> > V4L2_CID_MIN_BUFFERS_FOR_(CAPTURE/OUTPUT)
> > > - use existing controls for GOP size, number of frames and GOP closure
> > > - remove frame rate controls (in favour of the S_PARM call)
> > > - split level into separate controls for MPEG4 and H264
> > >
> > > I would welcome further comments.
> > >
> > > Best regards,
> > > Kamil Debski
> > >
> > > Signed-off-by: Kamil Debski 
> > > Signed-off-by: Kyungmin Park 
> > > ---
> > >  Documentation/DocBook/v4l/controls.xml |  733
> > 
> > >  include/linux/videodev2.h  |  147 +++
> > >  2 files changed, 880 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/Documentation/DocBook/v4l/controls.xml
> > b/Documentation/DocBook/v4l/controls.xml
> > > index 6880798..7c2df42 100644
> > > --- a/Documentation/DocBook/v4l/controls.xml
> > > +++ b/Documentation/DocBook/v4l/controls.xml
> > > @@ -325,6 +325,22 @@ minimum value disables backlight
> > compensation.
> > >  V4L2_CID_ILLUMINATORS_2 + 1).
> > > 
> > > 
> > > +
> 
> [snip]
> 
> > > +
> > > +   
> > > +   
> > > +  > spanname="id">V4L2_CID_MPEG_VIDEO_MAX_REF_PIC  > try>
> > > + boolean
> > > +   
> > > +   The maximum number of reference
> > pictures used for encoding.
> > > +   
> >
> > Is it boolean type?
> 
> It is not, a copy-paste mistake on my side.
> 
> [snip]
> 
> > > +
> > > +   
> > > +   
> > > +  > spanname="id">V4L2_CID_MPEG_VIDEO_H264_LOOP_FILTER_BETA
> >  
> > > + integer
> > > +   
> > > +   Loop filter alpha coefficient,
> > defined in the standard.
> > > +   
> >
> > alpha -> beta.
> 
> I agree.
> 
> [snip]
> 
> > > +
> > > +   
> > > +   
> > > +  > spanname="id">V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE 
> > 
> > > + boolean
> > > +   
> > > +   Padding enable - use a color
> > instead of repeating border pixels.
> > > +   
> >
> > The description may be wrong.
> 
> Thanks for pointing this out.
> 
> > > +
> > > +   
> > > +   
> > > +  > spanname="id">V4L2_CID_MPEG_VIDEO_H264_MB_RC_ENABLE&nbs
> > p;
> > > + boolean
> > > +   
> > > +   Macroblock level rate cont

RE: [RFC/PATCH v2] v4l: add control definitions for codec devices.

2011-06-02 Thread Kamil Debski
> From: Jeongtae Park [mailto:jtp.p...@samsung.com]
> Sent: 02 June 2011 09:44
> To: 'Kamil Debski'; linux-media@vger.kernel.org
> Cc: jaeryul...@samsung.com; june@samsung.com; janghyuck@samsung.com;
> kyungmin.p...@samsung.com; younglak1004@samsung.com; 'Marek Szyprowski'
> Subject: RE: [RFC/PATCH v2] v4l: add control definitions for codec devices.
> 
> Hi,
> 
> Thank you for your nice work. Here are my some comments.

Hi,

Thanks for your comments. I think I must have posted the wrong patch file...
I will send the proper one, with some of your suggestion in a minute.

> 
> > -Original Message-
> > From: Kamil Debski [mailto:k.deb...@samsung.com]
> > Sent: Tuesday, May 31, 2011 6:08 PM
> > Cc: m.szyprow...@samsung.com; kyungmin.p...@samsung.com;
> k.deb...@samsung.com; jaeryul...@samsung.com;
> > hverk...@xs4all.nl; laurent.pinch...@ideasonboard.com;
> jtp.p...@samsung.com
> > Subject: [RFC/PATCH v2] v4l: add control definitions for codec devices.
> >
> > Hi,
> >
> > This is a second version of the patch that adds controls for the codec
> family of
> > devices. I have implemented the suggestions I got from Hans Verkuil on the
> #v4l
> > channel.
> >
> > Change log:
> > - rename V4L2_CID_MIN_REQ_BUFS_(CAP/OUT) to
> V4L2_CID_MIN_BUFFERS_FOR_(CAPTURE/OUTPUT)
> > - use existing controls for GOP size, number of frames and GOP closure
> > - remove frame rate controls (in favour of the S_PARM call)
> > - split level into separate controls for MPEG4 and H264
> >
> > I would welcome further comments.
> >
> > Best regards,
> > Kamil Debski
> >
> > Signed-off-by: Kamil Debski 
> > Signed-off-by: Kyungmin Park 
> > ---
> >  Documentation/DocBook/v4l/controls.xml |  733
> 
> >  include/linux/videodev2.h  |  147 +++
> >  2 files changed, 880 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/DocBook/v4l/controls.xml
> b/Documentation/DocBook/v4l/controls.xml
> > index 6880798..7c2df42 100644
> > --- a/Documentation/DocBook/v4l/controls.xml
> > +++ b/Documentation/DocBook/v4l/controls.xml
> > @@ -325,6 +325,22 @@ minimum value disables backlight
> compensation.
> >  V4L2_CID_ILLUMINATORS_2 + 1).
> >   
> >   
> > +

[snip]

> > +
> > + 
> > + 
> > +spanname="id">V4L2_CID_MPEG_VIDEO_MAX_REF_PIC  try>
> > +   boolean
> > + 
> > + The maximum number of reference
> pictures used for encoding.
> > + 
> 
> Is it boolean type?

It is not, a copy-paste mistake on my side.

[snip]

> > +
> > + 
> > + 
> > +spanname="id">V4L2_CID_MPEG_VIDEO_H264_LOOP_FILTER_BETA
>  
> > +   integer
> > + 
> > + Loop filter alpha coefficient,
> defined in the standard.
> > + 
> 
> alpha -> beta.

I agree.

[snip]

> > +
> > + 
> > + 
> > +spanname="id">V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE 
> 
> > +   boolean
> > + 
> > + Padding enable - use a color
> instead of repeating border pixels.
> > + 
> 
> The description may be wrong.

Thanks for pointing this out.
 
> > +
> > + 
> > + 
> > +spanname="id">V4L2_CID_MPEG_VIDEO_H264_MB_RC_ENABLE&nbs
> p;
> > +   boolean
> > + 
> > + Macroblock level rate control
> enable for H264.
> > + 
> > +
> > + 
> > + 
> > +spanname="id">V4L2_CID_MPEG_VIDEO_FRAME_RATE_NOMINATOR&
> nbsp;
> > +   integer
> > + 
> > + Frames per second -
> nominator.
> > + 
> 
> Removed as you mentioned.

Yes, I think I had to mix up the patches that got sent.
 
> > +
> > + 
> > + 
> > +spanname="id">V4L2_CID_MPEG_VIDEO_FRAME_RATE_DENOMINATOR > 
> > +   integer
> > + 
> > + Frames per second -
> denominator
> > + 
> > +
> 
> Removed as you mentioned.

Ditto.
 
[snip]

> > +
> > + 
> > + 
> > +spanname="id">V4L2_CID_MPEG_VIDEO_VBV_BUF_SIZE  ntry>
> > +   integer
> > + 
> > + Quantization parameter for an P
> f

RE: [RFC/PATCH v2] v4l: add control definitions for codec devices.

2011-06-02 Thread Jeongtae Park
Hi,

Thank you for your nice work. Here are my some comments. 

> -Original Message-
> From: Kamil Debski [mailto:k.deb...@samsung.com]
> Sent: Tuesday, May 31, 2011 6:08 PM
> Cc: m.szyprow...@samsung.com; kyungmin.p...@samsung.com; 
> k.deb...@samsung.com; jaeryul...@samsung.com;
> hverk...@xs4all.nl; laurent.pinch...@ideasonboard.com; jtp.p...@samsung.com
> Subject: [RFC/PATCH v2] v4l: add control definitions for codec devices.
> 
> Hi,
> 
> This is a second version of the patch that adds controls for the codec family 
> of
> devices. I have implemented the suggestions I got from Hans Verkuil on the 
> #v4l
> channel.
> 
> Change log:
> - rename V4L2_CID_MIN_REQ_BUFS_(CAP/OUT) to 
> V4L2_CID_MIN_BUFFERS_FOR_(CAPTURE/OUTPUT)
> - use existing controls for GOP size, number of frames and GOP closure
> - remove frame rate controls (in favour of the S_PARM call)
> - split level into separate controls for MPEG4 and H264
> 
> I would welcome further comments.
> 
> Best regards,
> Kamil Debski
> 
> Signed-off-by: Kamil Debski 
> Signed-off-by: Kyungmin Park 
> ---
>  Documentation/DocBook/v4l/controls.xml |  733 
> 
>  include/linux/videodev2.h  |  147 +++
>  2 files changed, 880 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/DocBook/v4l/controls.xml 
> b/Documentation/DocBook/v4l/controls.xml
> index 6880798..7c2df42 100644
> --- a/Documentation/DocBook/v4l/controls.xml
> +++ b/Documentation/DocBook/v4l/controls.xml
> @@ -325,6 +325,22 @@ minimum value disables backlight compensation.
>  V4L2_CID_ILLUMINATORS_2 + 1).
> 
> 
> + V4L2_CID_MIN_BUFFERS_FOR_CAPTURE
> + integer
> + This is a read only control that can read by the application
> +and used as a hint to determine the number of CAPTURE buffer to pass to 
> REQBUFS.
> +The value is the minimum number of CAPTURE buffer that it necessary for 
> hardware
> +to work.
> +   
> +   
> + V4L2_CID_MIN_BUFFERSS_FOR_OUTPUT
> + integer
> + This is a read only control that can read by the application
> +and used as a hint to determine the number of OUTPUT buffer to pass to 
> REQBUFS.
> +The value is the minimum number of OUTPUT buffer that it necessary for 
> hardware
> +to work.
> +   
> +   
>   V4L2_CID_PRIVATE_BASE
>   
>   ID of the first custom (driver specific) control.
> @@ -1409,6 +1425,723 @@ of the video. The supplied 32-bit integer is 
> interpreted as follows (bit
> 
>   
> 
> +
> +
> +   
> +   
> +  spanname="id">V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE 
> + boolean
> +   
> +   If enabled the decoder expects a 
> single slice in one buffer, otherwise
> +the decoder expects a single frame in one input buffer.
> +   
> +
> +   
> +   
> +  spanname="id">V4L2_CID_MPEG_VIDEO_H264_VUI_AR_ENABLE 
> + boolean
> +   
> +   Enable writing aspect ratio in the 
> Video Usability Information.
> +   
> +
> +   
> +   
> +  spanname="id">V4L2_CID_MPEG_VIDEO_H264_VUI_AR_IDC 
> + integer
> +   
> +   VUI aspect ratio IDC for H.264 
> encoding. The value is defined in VUI Table
> +E-1 in the standard.
> +   
> +   
> +
> +   
> +   
> +  spanname="id">V4L2_CID_MPEG_VIDEO_H264_EXT_SAR_WIDTH 
> + integer
> +   
> +   Extended sample aspect ratio width 
> for H.264 VUI encoding.
> +   
> +
> +   
> +   
> +  spanname="id">V4L2_CID_MPEG_VIDEO_H264_EXT_SAR_HEIGHT 
> + integer
> +   
> +   Extended sample aspect ratio height 
> for H.264 VUI encoding.
> +   
> +
> +   
> +   
> +  spanname="id">V4L2_CID_MPEG_VIDEO_LEVEL 
> + enum v4l2_mpeg_level
> +   
> +   The level information for stream.
> +Possible values are:
> +   
> +   
> + 
> +   
> + 
> +   
> V4L2_MPEG_VIDEO_LEVEL_0 
> +   Level 0
> + 
> + 
> +   
> V4L2_MPEG_VIDEO_LEVEL_0B 
> +   Level 0b
> + 
> + 
> +   
> V4L2_MPEG_VIDEO_LEVEL_1 
> +   Level 1.0
> + 
> + 
> +   
> V4L2_MPEG_VIDEO_LEVEL_1_1 
> +   Level 1.1
> + 
> + 
> +   
> V4L2_MPEG_VIDEO_LEVEL_1_2 
> +   Level 1.2
> + 
> + 
> +   
> V4L2_MPEG_VIDEO_LEVEL_1_3 
> +   Level 1.3
> + 
> + 
> +   
> V4L2_MPEG_VIDEO_LEVEL_2 
>