From mboxrd@z Thu Jan 1 00:00:00 1970 From: k.debski@samsung.com (Kamil Debski) Date: Thu, 30 Jan 2014 16:22:18 +0100 Subject: [PATCH v2 1/2] drivers/media: v4l2: Add settings for Horizontal and Vertical MV Search Range In-Reply-To: <52EA00E5.10303@xs4all.nl> References: <52E0ED10.2020901@samsung.com> <1391060563-27015-1-git-send-email-amit.grover@samsung.com> <1391060563-27015-2-git-send-email-amit.grover@samsung.com> <52EA00E5.10303@xs4all.nl> Message-ID: <051c01cf1dcf$1132ee60$3398cb20$%debski@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Hans, Amit, > From: Hans Verkuil [mailto:hverkuil at xs4all.nl] > Sent: Thursday, January 30, 2014 8:36 AM > > On 01/30/2014 06:42 AM, Amit Grover wrote: > > Adding V4L2 controls for horizontal and vertical search range in > > pixels for motion estimation module in video encoder. > > > > Signed-off-by: Swami Nathan > > Signed-off-by: Amit Grover > > --- > > Documentation/DocBook/media/v4l/controls.xml | 20 > ++++++++++++++++++++ > > drivers/media/v4l2-core/v4l2-ctrls.c | 14 ++++++++++++++ > > include/uapi/linux/v4l2-controls.h | 2 ++ > > 3 files changed, 36 insertions(+) > > > > diff --git a/Documentation/DocBook/media/v4l/controls.xml > > b/Documentation/DocBook/media/v4l/controls.xml > > index 7a3b49b..be04d18 100644 > > --- a/Documentation/DocBook/media/v4l/controls.xml > > +++ b/Documentation/DocBook/media/v4l/controls.xml > > @@ -2258,6 +2258,26 @@ Applicable to the MPEG1, MPEG2, MPEG4 > > encoders. VBV buffer control. > > > > > > + > > + > > + spanname="id">V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE >  > > + integer > > + > > + Horizontal search range > defines > > +maximum horizontal search area in pixels to search and match for the > > +present Macroblock (MB) in the reference picture. This V4L2 control > macro is used to set horizontal search range for motion estimation > module in video encoder. > > + > > + > > + > > + > > + spanname="id">V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE >  > > + integer > > These two controls sound very mfc specific as opposed to being part of > the standard. > If so, then they should be named V4L2_CID_MPEG_MFC51_*. mencoder has an option to set motion vector search radius. >>From man mencoder: me_range=<4-64> radius of exhaustive or multi-hexagon motion search (default: 16) So I think it could be applicable to other hardware codecs as well. > Also, for which codecs are these controls applicable? Isn't the choice of motion estimation algorithm (and its parameters) codec agnostic, as long as the restrictions of the standard are honoured? > > > + > > + Vertical search range defines > maximum > > +vertical search area in pixels to search and match for the present > > +Macroblock (MB) in the reference picture. This V4L2 control macro is > used to set vertical search range for motion estimation module in video > encoder. > > + > > + > > > > > > > > spanname="id">V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE&n > > bsp; diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c > > b/drivers/media/v4l2-core/v4l2-ctrls.c > > index fb46790..e775388 100644 > > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > > @@ -735,6 +735,8 @@ const char *v4l2_ctrl_get_name(u32 id) > > 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 Control"; > > + case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE: return > "Horizontal MV Search Range"; > > + case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE: return > "Vertical MV Search Range"; > > case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER: return > "Repeat Sequence Header"; > > > > /* VPX controls */ > > @@ -905,6 +907,18 @@ void v4l2_ctrl_fill(u32 id, const char **name, > enum v4l2_ctrl_type *type, > > *min = 0; > > *max = *step = 1; > > break; > > + case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE: > > + *type = V4L2_CTRL_TYPE_INTEGER; > > + *min = 16; > > + *max = 128; > > + *step = 16; > > Weird range, why not use range 1-8? If the search range is represented in pixels, then pixels should be used. It the control is also used in other hardware, then I think it the values should not be limited in this way here. For example mencoder accepts the value between 4 and 64. The range and step could be limited in the s5p_mfc_enc.c. > > > + break; > > + case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE: > > + *type = V4L2_CTRL_TYPE_INTEGER; > > + *min = 16; > > + *max = 128; > > + *step = 16; > > + break; > > case V4L2_CID_PAN_RESET: > > case V4L2_CID_TILT_RESET: > > case V4L2_CID_FLASH_STROBE: > > diff --git a/include/uapi/linux/v4l2-controls.h > > b/include/uapi/linux/v4l2-controls.h > > index 1666aab..80e1def 100644 > > --- a/include/uapi/linux/v4l2-controls.h > > +++ b/include/uapi/linux/v4l2-controls.h > > @@ -372,6 +372,8 @@ enum v4l2_mpeg_video_multi_slice_mode { > > #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_REPEAT_SEQ_HEADER > (V4L2_CID_MPEG_BASE+226) > > +#define V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE > (V4L2_CID_MPEG_BASE+227) > > +#define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE > (V4L2_CID_MPEG_BASE+228) > > > > #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) > > > > Regards, > > Hans Best wishes, -- Kamil Debski Samsung R&D Institute Poland