RE: [PATCH 1/3] v4l: add contorl definitions for codec devices.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans, hi Jeongtae,

> From: Hans Verkuil [mailto:hverkuil@xxxxxxxxx]
> Sent: 02 March 2012 09:13
> 
> Hi Jeongtae!
> 
> Some review notes below...
> 
> On Friday, March 02, 2012 03:17:40 Jeongtae Park wrote:
> > Add control definitions for controls specific to codec devices.
> >
> > Signed-off-by: Jeongtae Park <jtp.park@xxxxxxxxxxx>
> > Cc: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> > Cc: Kamil Debski <k.debski@xxxxxxxxxxx>
> > ---
> >  include/linux/videodev2.h |   51
> ++++++++++++++++++++++++++++++++++++++++++--
> >  1 files changed, 48 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index b739d7d..a19512a 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -359,7 +359,9 @@ struct v4l2_pix_format {
> >
> >  /* two non contiguous planes - one Y, one Cr + Cb interleaved  */
> >  #define V4L2_PIX_FMT_NV12M   v4l2_fourcc('N', 'M', '1', '2') /* 12
> Y/CbCr 4:2:0  */
> > +#define V4L2_PIX_FMT_NV21M   v4l2_fourcc('N', 'M', '2', '1') /* 21
> Y/CrCb 4:2:0  */
> >  #define V4L2_PIX_FMT_NV12MT  v4l2_fourcc('T', 'M', '1', '2') /* 12
> Y/CbCr 4:2:0 64x32 macroblocks */
> > +#define V4L2_PIX_FMT_NV12MT_16X16 v4l2_fourcc('V', 'M', '1', '2') /* 12
> Y/CbCr 4:2:0 16x16 macroblocks */
> >
> >  /* three non contiguous planes - Y, Cb, Cr */
> >  #define V4L2_PIX_FMT_YUV420M v4l2_fourcc('Y', 'M', '1', '2') /* 12
> YUV420 planar */
> > @@ -392,6 +394,7 @@ struct v4l2_pix_format {
> >  #define V4L2_PIX_FMT_MPEG     v4l2_fourcc('M', 'P', 'E', 'G') /* MPEG-
> 1/2/4 Multiplexed */
> >  #define V4L2_PIX_FMT_H264     v4l2_fourcc('H', '2', '6', '4') /* H264
> with start codes */
> >  #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264
> without start codes */
> > +#define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC
> */
> >  #define V4L2_PIX_FMT_H263     v4l2_fourcc('H', '2', '6', '3') /* H263
> */
> >  #define V4L2_PIX_FMT_MPEG1    v4l2_fourcc('M', 'P', 'G', '1') /* MPEG-1
> ES     */
> >  #define V4L2_PIX_FMT_MPEG2    v4l2_fourcc('M', 'P', 'G', '2') /* MPEG-2
> ES     */
> > @@ -399,6 +402,7 @@ struct v4l2_pix_format {
> >  #define V4L2_PIX_FMT_XVID     v4l2_fourcc('X', 'V', 'I', 'D') /* Xvid
> */
> >  #define V4L2_PIX_FMT_VC1_ANNEX_G v4l2_fourcc('V', 'C', '1', 'G') /* SMPTE
> 421M Annex G compliant stream */
> >  #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE
> 421M Annex L compliant stream */
> > +#define V4L2_PIX_FMT_VP8      v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
> 
> Note that these new formats need to be documented in the spec as well.
> 
> >
> >  /*  Vendor-specific formats   */
> >  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1
> YUV */
> > @@ -1458,17 +1462,18 @@ enum v4l2_mpeg_video_header_mode {
> >  };
> >  #define V4L2_CID_MPEG_VIDEO_MAX_REF_PIC
> 	(V4L2_CID_MPEG_BASE+217)
> >  #define V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE
> 	(V4L2_CID_MPEG_BASE+218)
> > -#define V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_BYTES
> 	(V4L2_CID_MPEG_BASE+219)
> > +#define V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_BITS
> 	(V4L2_CID_MPEG_BASE+219)
> 
> Why change from bytes to bits? That changes the meaning of this control.

We had a discussion about this on irc with Hans. Let me paste a snippet:

Jun 08 09:58:36 <kdebski>	hverkuil: in the first email you have wrote that you would like to add some more comments on the codec controls after you get yourself familiar with the standard. any progress on that?
Jun 08 10:06:17 <hverkuil>	kdebski: yes, I want to go through the docbook patch again and check each item against the standard docs I have.
Jun 08 10:06:48 <hverkuil>	I'll just make notes here as I go along and you can comment on it.
Jun 08 10:08:02 <hverkuil>	I want to finish my review today if possible.
Jun 08 10:10:12 <hverkuil>	kdebski: VIDEO_MAX_REF_PIC: is this H264 specific or is it also valid for MPEG4?
Jun 08 10:18:36 <hverkuil>	kdebski: VIDEO_MULTI_SLICE_MODE: MAX_BITS: is this specific to your encoder, or is this part of the standard? And why is it in bits instead of bytes?
Jun 08 10:22:01 <kdebski>	I have had a look at the most recent documentation for MFC and MAX_REF_PIC disappeared, previously I have categorised it as H264 only
Jun 08 10:24:15 <kdebski>	in ffmpeg documentation it seems as a control that can be used in other codecs as well
Jun 08 10:25:37 <kdebski>	VIDEO_MULTI_SLICE_MODE: MAX_BITS it can be set in x264 with --slice-max-size (it is in bytes there)
Jun 08 10:26:10 <hverkuil>	Ah, so that slice mode is for x264, not h264?
Jun 08 10:26:33 <kdebski>	hverkuil: for MFC it is bits
Jun 08 10:26:56 <kdebski>	x264 is an open source h264 encoder
Jun 08 10:28:20 <hverkuil>	so with MAX_BITS you can get variable number of MBs per slice, right?
Jun 08 10:28:56 <kdebski>	yes
Jun 08 10:29:00 <hverkuil>	I think I'd call it MAX_BYTES.

Although I wasn't too convinced whether it should be BITS or BYTES we have made a choice. It is in the mainline kernel. It is part of the API - changing it now could and probably will brake existing applications.
The idea was to model the controls on the arguments/parameters used by existing open source codecs, such as x264.
 
[snip]

Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center

--
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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux