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

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

Hi Jongtae,

On 03/02/2012 03:17 AM, Jeongtae Park wrote:
> @@ -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)
>  #define V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_MB		(V4L2_CID_MPEG_BASE+220)
>  #define V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE		(V4L2_CID_MPEG_BASE+221)
>  enum v4l2_mpeg_video_multi_slice_mode {
>  	V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_SINGLE		= 0,
> -	V4L2_MPEG_VIDEO_MULTI_SICE_MODE_MAX_MB		= 1,
> -	V4L2_MPEG_VIDEO_MULTI_SICE_MODE_MAX_BYTES	= 2,
> +	V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_MAX_MB		= 1,
> +	V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_MAX_BITS	= 2,

Should this be a separate bugfix patch ? We need to make sure the 
applications work with older kernels too, perhaps a patch with 
stable@vger.kernel.org at Cc would do.

Don't you need to update the multi_slice[] array in v4l2-ctrls.c 
as well ? Now it is:

static const char * const multi_slice[] = {
	"Single",
	"Max Macroblocks",
	"Max Bytes",
	NULL,
}; 

And the corresponding individual controls:

 case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_BYTES: return "Maximum Bytes in a Slice";
 case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_MB:	 return "Number of MBs in a Slice";

Please make sure the descriptions are consistent with CIDs.

--

Regards,
Sylwester