From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp-vbr9.xs4all.nl ([194.109.24.29]:2760 "EHLO smtp-vbr9.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751447Ab2JAO1r (ORCPT ); Mon, 1 Oct 2012 10:27:47 -0400 From: Hans Verkuil To: Arun Kumar K Subject: Re: [PATCH v8 2/6] [media] v4l: Add control definitions for new H264 encoder features Date: Mon, 1 Oct 2012 16:27:26 +0200 Cc: linux-media@vger.kernel.org, k.debski@samsung.com, jtp.park@samsung.com, janghyuck.kim@samsung.com, jaeryul.oh@samsung.com, ch.naveen@samsung.com, m.szyprowski@samsung.com, s.nawrocki@samsung.com, kmpark@infradead.org, joshi@samsung.com References: <1349129099-6480-1-git-send-email-arun.kk@samsung.com> <1349129099-6480-3-git-send-email-arun.kk@samsung.com> In-Reply-To: <1349129099-6480-3-git-send-email-arun.kk@samsung.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201210011627.26644.hverkuil@xs4all.nl> Sender: linux-media-owner@vger.kernel.org List-ID: Hi Arun, I've got a bunch of comments below, all pretty much small stuff... On Tue October 2 2012 00:04:55 Arun Kumar K wrote: > New controls are added for supporting H264 encoding features like > - MVC frame packing > - Flexible macroblock ordering > - Arbitrary slice ordering > - Hierarchial coding > > Signed-off-by: Jeongtae Park > Signed-off-by: Naveen Krishna Chatradhi > Signed-off-by: Arun Kumar K > --- > Documentation/DocBook/media/v4l/controls.xml | 268 +++++++++++++++++++++++++- > drivers/media/v4l2-core/v4l2-ctrls.c | 42 ++++ > include/linux/v4l2-controls.h | 41 ++++ > 3 files changed, 350 insertions(+), 1 deletions(-) > > diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml > index 272a5f7..ce2cfd3 100644 > --- a/Documentation/DocBook/media/v4l/controls.xml > +++ b/Documentation/DocBook/media/v4l/controls.xml > @@ -1586,7 +1586,6 @@ frame counter of the frame that is currently displayed (decoded). This value is > the decoder is started. > > > - > > > V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE  > @@ -2270,6 +2269,14 @@ Applicable to the MPEG1, MPEG2, MPEG4 encoders. > > > > + > + V4L2_CID_MPEG_VIDEO_VBV_DELAY  > + integer > + Sets the initial delay in milliseconds for > +VBV buffer control. > + > + > + > > V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE  > integer > @@ -2334,6 +2341,265 @@ Applicable to the MPEG4 decoder. > vop_time_increment value for MPEG4. Applicable to the MPEG4 encoder. > > > + > + > + V4L2_CID_MPEG_VIDEO_H264_SEI_FRAME_PACKING  > + boolean > + > + Enable generation of frame packing supplemental enhancement information in the encoded bitstream. > +The frame packing SEI message contains the arrangement of L and R planes for 3D viewing. Applicable to the H264 encoder. > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_SEI_FP_CURRENT_FRAME_0  > + boolean > + > + Sets current frame as frame0 in frame packing SEI. > +Applicable to the H264 encoder. > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE  > + enum v4l2_mpeg_video_h264_sei_fp_arrangement_type > + > + Frame packing arrangement type for H264 SEI. > +Applicable to the H264 encoder. > +Possible values are: > + > + > + > + > + > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_CHEKERBOARD  > + Pixels are alternatively from L and R. > + > + > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_COLUMN  > + L and R are interlaced by column. > + > + > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_ROW  > + L and R are interlaced by row. > + > + > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_SIDE_BY_SIDE  > + L is on the left, R on the right. > + > + > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_TOP_BOTTOM  > + L is on top, R on bottom. > + > + > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_TEMPORAL  > + One view per frame. > + > + > + > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_FMO  > + boolean > + > + Enables flexible macroblock ordering in the encoded bitstream. It is a technique > +used for restructuring the ordering of macroblocks in pictures. Applicable to the H264 encoder. > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_FMO_MAP_TYPE  > + enum v4l2_mpeg_video_h264_fmo_map_type > + > + When using FMO, the map type divides the image in different scan patterns of macroblocks. > +Applicable to the H264 encoder. > +Possible values are: > + > + > + > + > + > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_INTERLEAVED_SLICES  > + Slices are interleaved one after other with macroblocks in run length order. > + > + > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_SCATTERED_SLICES  > + Scatters the macroblocks based on a mathematical function known to both encoder and decoder. > + > + > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_FOREGROUND_WITH_LEFT_OVER  > + Macroblocks arranged in rectangular areas or regions of interest. > + > + > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_BOX_OUT  > + Slice groups grow in a cyclic way from centre to outwards. > + > + > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_RASTER_SCAN  > + Slice groups grow in raster scan pattern from left to right. > + > + > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_WIPE_SCAN  > + Slice groups grow in wipe scan pattern from top to bottom. > + > + > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_EXPLICIT  > + User defined map type. > + > + > + > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_FMO_SLICE_GROUP  > + integer > + > + Number of slice groups in FMO. > +Applicable to the H264 encoder. > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_FMO_CHANGE_DIRECTION  > + enum v4l2_mpeg_video_h264_fmo_change_dir > + > + Specifies a direction of the slice group change for raster and wipe maps. > +Applicable to the H264 encoder. > +Possible values are: > + > + > + > + > + > + V4L2_MPEG_VIDEO_H264_FMO_CHANGE_DIR_RIGHT  > + Raster scan or wipe right. > + > + > + V4L2_MPEG_VIDEO_H264_FMO_CHANGE_DIR_LEFT  > + Reverse raster scan or wipe left. > + > + > + > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_FMO_CHANGE_RATE  > + integer > + > + Specifies the size of the first slice group for raster and wipe map. > +Applicable to the H264 encoder. > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_FMO_RUN_LENGTH  > + integer > + > + Specifies the number of consecutive macroblocks for the interleaved map. > +Applicable to the H264 encoder. > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_ASO  > + boolean > + > + Enables arbitrary slice ordering in encoded bitstream. > +Applicable to the H264 encoder. > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_ASO_SLICE_ORDER  > + integer > + Specifies the slice order in ASO. Applicable to the H264 encoder. > +The supplied 32-bit integer is interpreted as follows (bit > +0 = least significant bit): > + > + > + > + > + > + Bit 0:15 > + Slice ID > + > + > + Bit 16:32 > + Slice position or order > + > + > + > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING  > + boolean > + > + Enables H264 hierarchial coding. hierarchial -> hierarchical This same typo is made in quite a few places, so please to a case-insensitive search for this. > +Applicable to the H264 encoder. > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_TYPE  > + enum v4l2_mpeg_video_h264_hierarchical_coding_type > + > + Specifies the hierarchial coding type. > +Applicable to the H264 encoder. > +Possible values are: > + > + > + > + > + > + V4L2_MPEG_VIDEO_H264_HIERARCHICAL_CODING_B  > + Hierarchial B coding. > + > + > + V4L2_MPEG_VIDEO_H264_HIERARCHICAL_CODING_P  > + Hierarchial P coding. > + > + > + > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER  > + integer > + > + Specifies the number of hierarchial coding layers. > +Applicable to the H264 encoder. > + > + > + > + > + V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP  > + integer > + Specifies a user defined QP for each layer. Applicable to the H264 encoder. > +The supplied 32-bit integer is interpreted as follows (bit > +0 = least significant bit): > + > + > + > + > + > + Bit 0:15 > + QP value > + > + > + Bit 16:32 > + Layer number > + > + > + > + > + > > > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c > index f400035..a7518cb 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -384,6 +384,25 @@ const char * const *v4l2_ctrl_get_menu(u32 id) > "Extended SAR", > NULL, > }; > + static const char * const h264_fp_arrangement_type[] = { > + "Checkerboard", > + "Column", > + "Row", > + "Side by side", "Side by Side" > + "Top Bottom", > + "Temporal", > + NULL, > + }; > + static const char * const h264_fmo_map_type[] = { > + "Interleaved Slices", > + "Scattered Slices", > + "Foreground With Leftover", With -> with I can't help the weird capitalization rules in English w.r.t. titles :-) > + "Box Out", > + "Raster Scan", > + "Wipe Scan", > + "Explicit", > + NULL, > + }; > static const char * const mpeg_mpeg4_level[] = { > "0", > "0b", > @@ -508,6 +527,10 @@ const char * const *v4l2_ctrl_get_menu(u32 id) > return h264_profile; > case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC: > return vui_sar_idc; > + case V4L2_CID_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE: > + return h264_fp_arrangement_type; > + case V4L2_CID_MPEG_VIDEO_H264_FMO_MAP_TYPE: > + return h264_fmo_map_type; > case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL: > return mpeg_mpeg4_level; > case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE: > @@ -643,6 +666,22 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_MPEG_VIDEO_H264_VUI_EXT_SAR_WIDTH: return "Horizontal Size of SAR"; > case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_ENABLE: return "Aspect Ratio VUI Enable"; > case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC: return "VUI Aspect Ratio IDC"; > + case V4L2_CID_MPEG_VIDEO_H264_SEI_FRAME_PACKING: return "H264 Enable Frame Packing SEI"; > + case V4L2_CID_MPEG_VIDEO_H264_SEI_FP_CURRENT_FRAME_0: return "H264 Set Current Frame as Frame0"; > + case V4L2_CID_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE: return "H264 Frame Packing Arrangement Type"; > + case V4L2_CID_MPEG_VIDEO_H264_FMO: return "H264 Flexible Macroblock Ordering"; > + case V4L2_CID_MPEG_VIDEO_H264_FMO_MAP_TYPE: return "H264 Map Type for FMO"; > + case V4L2_CID_MPEG_VIDEO_H264_FMO_SLICE_GROUP: return "H264 FMO Number of Slice Groups"; > + case V4L2_CID_MPEG_VIDEO_H264_FMO_CHANGE_DIRECTION: return "H264 FMO Direction of the Slice Group Change"; > + case V4L2_CID_MPEG_VIDEO_H264_FMO_CHANGE_RATE: return "H264 FMO Size of the First Slice Group"; > + case V4L2_CID_MPEG_VIDEO_H264_FMO_RUN_LENGTH: return "H264 FMO Number of Consecutive MBs"; > + case V4L2_CID_MPEG_VIDEO_H264_ASO: return "H264 Arbitrary Slice Ordering"; > + case V4L2_CID_MPEG_VIDEO_H264_ASO_SLICE_ORDER: return "H264 ASO Slice Order"; > + case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING: return "Enable H264 Hierarchial Coding"; > + case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_TYPE: return "H264 Hierarchial Coding Type"; > + case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER:return "H264 Number of Hierarchial Coding Layers"; > + case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP: > + return "H264 Set QP Value for Hierarchial Coding Layers"; Note that the string can only be 32 chars long (including the terminating zero), so quite a few of these strings are too long. The control framework will just cut off the string. > case V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP: return "MPEG4 I-Frame QP Value"; > case V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP: return "MPEG4 P-Frame QP Value"; > case V4L2_CID_MPEG_VIDEO_MPEG4_B_FRAME_QP: return "MPEG4 B-Frame QP Value"; > @@ -657,6 +696,7 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_MPEG_VIDEO_VBV_SIZE: return "VBV Buffer Size"; > case V4L2_CID_MPEG_VIDEO_DEC_PTS: return "Video Decoder PTS"; > case V4L2_CID_MPEG_VIDEO_DEC_FRAME: return "Video Decoder Frame Count"; > + case V4L2_CID_MPEG_VIDEO_VBV_DELAY: return "Initial Delay for VBV Buffer Control"; > > /* CAMERA controls */ > /* Keep the order of the 'case's the same as in videodev2.h! */ > @@ -853,6 +893,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_MPEG_VIDEO_H264_LOOP_FILTER_MODE: > case V4L2_CID_MPEG_VIDEO_H264_PROFILE: > case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC: > + case V4L2_CID_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE: > + case V4L2_CID_MPEG_VIDEO_H264_FMO_MAP_TYPE: > case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL: > case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE: > case V4L2_CID_JPEG_CHROMA_SUBSAMPLING: > diff --git a/include/linux/v4l2-controls.h b/include/linux/v4l2-controls.h > index 421d24c..86e79af 100644 > --- a/include/linux/v4l2-controls.h > +++ b/include/linux/v4l2-controls.h > @@ -349,6 +349,7 @@ enum v4l2_mpeg_video_multi_slice_mode { > #define V4L2_CID_MPEG_VIDEO_VBV_SIZE (V4L2_CID_MPEG_BASE+222) > #define V4L2_CID_MPEG_VIDEO_DEC_PTS (V4L2_CID_MPEG_BASE+223) > #define V4L2_CID_MPEG_VIDEO_DEC_FRAME (V4L2_CID_MPEG_BASE+224) > +#define V4L2_CID_MPEG_VIDEO_VBV_DELAY (V4L2_CID_MPEG_BASE+225) > > #define V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP (V4L2_CID_MPEG_BASE+300) > #define V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP (V4L2_CID_MPEG_BASE+301) > @@ -439,6 +440,46 @@ enum v4l2_mpeg_video_h264_vui_sar_idc { > V4L2_MPEG_VIDEO_H264_VUI_SAR_IDC_2x1 = 16, > V4L2_MPEG_VIDEO_H264_VUI_SAR_IDC_EXTENDED = 17, > }; > +#define V4L2_CID_MPEG_VIDEO_H264_SEI_FRAME_PACKING (V4L2_CID_MPEG_BASE+368) > +#define V4L2_CID_MPEG_VIDEO_H264_SEI_FP_CURRENT_FRAME_0 (V4L2_CID_MPEG_BASE+369) > +#define V4L2_CID_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE (V4L2_CID_MPEG_BASE+370) > +enum v4l2_mpeg_video_h264_sei_fp_arrangement_type { > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_CHEKERBOARD = 0, Typo: CHEKER -> CHECKER > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_COLUMN = 1, > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_ROW = 2, > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_SIDE_BY_SIDE = 3, > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_TOP_BOTTOM = 4, > + V4L2_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE_TEMPORAL = 5, > +}; > +#define V4L2_CID_MPEG_VIDEO_H264_FMO (V4L2_CID_MPEG_BASE+371) > +#define V4L2_CID_MPEG_VIDEO_H264_FMO_MAP_TYPE (V4L2_CID_MPEG_BASE+372) > +enum v4l2_mpeg_video_h264_fmo_map_type { > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_INTERLEAVED_SLICES = 0, > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_SCATTERED_SLICES = 1, > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_FOREGROUND_WITH_LEFT_OVER = 2, > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_BOX_OUT = 3, > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_RASTER_SCAN = 4, > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_WIPE_SCAN = 5, > + V4L2_MPEG_VIDEO_H264_FMO_MAP_TYPE_EXPLICIT = 6, > +}; > +#define V4L2_CID_MPEG_VIDEO_H264_FMO_SLICE_GROUP (V4L2_CID_MPEG_BASE+373) > +#define V4L2_CID_MPEG_VIDEO_H264_FMO_CHANGE_DIRECTION (V4L2_CID_MPEG_BASE+374) > +enum v4l2_mpeg_video_h264_fmo_change_dir { > + V4L2_MPEG_VIDEO_H264_FMO_CHANGE_DIR_RIGHT = 0, > + V4L2_MPEG_VIDEO_H264_FMO_CHANGE_DIR_LEFT = 1, > +}; > +#define V4L2_CID_MPEG_VIDEO_H264_FMO_CHANGE_RATE (V4L2_CID_MPEG_BASE+375) > +#define V4L2_CID_MPEG_VIDEO_H264_FMO_RUN_LENGTH (V4L2_CID_MPEG_BASE+376) > +#define V4L2_CID_MPEG_VIDEO_H264_ASO (V4L2_CID_MPEG_BASE+377) > +#define V4L2_CID_MPEG_VIDEO_H264_ASO_SLICE_ORDER (V4L2_CID_MPEG_BASE+378) > +#define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING (V4L2_CID_MPEG_BASE+379) > +#define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_TYPE (V4L2_CID_MPEG_BASE+380) > +enum v4l2_mpeg_video_h264_hierarchical_coding_type { > + V4L2_MPEG_VIDEO_H264_HIERARCHICAL_CODING_B = 0, > + V4L2_MPEG_VIDEO_H264_HIERARCHICAL_CODING_P = 1, > +}; > +#define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER (V4L2_CID_MPEG_BASE+381) > +#define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP (V4L2_CID_MPEG_BASE+382) > #define V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP (V4L2_CID_MPEG_BASE+400) > #define V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP (V4L2_CID_MPEG_BASE+401) > #define V4L2_CID_MPEG_VIDEO_MPEG4_B_FRAME_QP (V4L2_CID_MPEG_BASE+402) > Regards, Hans