V4L/DVB (10277): cx18, cx2341x: Fix bugs in cx18 AC3 control and comp…

…ly with V4L2 spec Fix bugs in the cx18 AC3 control implementation that would have affected ivtv and other drivers via the cx2341x module. Bring AC3 controls behavior into comliance with V4L2 specification. Thanks to Hans Verkuil for reviewing the previous patch and pointing out the problems. Reported-by: Hans Verkuil <hverkuil@xs4all.nl> Signed-off-by: Andy Walls <awalls@radix.net> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
git-mirror · Mar 30, 2009 · 31230c5 · 31230c5
1 parent 0d82fe8
commit 31230c5
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 14 deletions.
diff --git a/drivers/media/video/cx18/cx18-driver.c b/drivers/media/video/cx18/cx18-driver.c
@@ -587,8 +587,7 @@ static int __devinit cx18_init_struct1(struct cx18 *cx)
 		(cx->params.video_temporal_filter_mode << 1) |
 		(cx->params.video_median_filter_type << 2);
 	cx->params.port = CX2341X_PORT_MEMORY;
-	cx->params.capabilities = CX2341X_CAP_HAS_TS   | CX2341X_CAP_HAS_AC3 |
-				  CX2341X_CAP_HAS_LPCM;
+	cx->params.capabilities = CX2341X_CAP_HAS_TS | CX2341X_CAP_HAS_AC3;
 	init_waitqueue_head(&cx->cap_w);
 	init_waitqueue_head(&cx->mb_apu_waitq);
 	init_waitqueue_head(&cx->mb_cpu_waitq);

diff --git a/drivers/media/video/cx2341x.c b/drivers/media/video/cx2341x.c
@@ -263,10 +263,10 @@ static int cx2341x_set_ctrl(struct cx2341x_mpeg_params *params, int busy,
 	case V4L2_CID_MPEG_AUDIO_ENCODING:
 		if (busy)
 			return -EBUSY;
-		if (params->capabilities & CX2341X_CAP_HAS_AC3 &&
-		    ctrl->value != V4L2_MPEG_AUDIO_ENCODING_LAYER_2 &&
-		    ctrl->value != V4L2_MPEG_AUDIO_ENCODING_AC3)
-			return -EINVAL;
+		if (params->capabilities & CX2341X_CAP_HAS_AC3)
+			if (ctrl->value != V4L2_MPEG_AUDIO_ENCODING_LAYER_2 &&
+			    ctrl->value != V4L2_MPEG_AUDIO_ENCODING_AC3)
+				return -ERANGE;
 		params->audio_encoding = ctrl->value;
 		break;
 	case V4L2_CID_MPEG_AUDIO_L2_BITRATE:
@@ -277,6 +277,8 @@ static int cx2341x_set_ctrl(struct cx2341x_mpeg_params *params, int busy,
 	case V4L2_CID_MPEG_AUDIO_AC3_BITRATE:
 		if (busy)
 			return -EBUSY;
+		if (!(params->capabilities & CX2341X_CAP_HAS_AC3))
+			return -EINVAL;
 		params->audio_ac3_bitrate = ctrl->value;
 		break;
 	case V4L2_CID_MPEG_AUDIO_MODE:
@@ -498,32 +500,54 @@ int cx2341x_ctrl_query(const struct cx2341x_mpeg_params *params,
 
 	switch (qctrl->id) {
 	case V4L2_CID_MPEG_AUDIO_ENCODING:
-		if (params->capabilities & CX2341X_CAP_HAS_AC3)
+		if (params->capabilities & CX2341X_CAP_HAS_AC3) {
+			/*
+			 * The state of L2 & AC3 bitrate controls can change
+			 * when this control changes, but v4l2_ctrl_query_fill()
+			 * already sets V4L2_CTRL_FLAG_UPDATE for
+			 * V4L2_CID_MPEG_AUDIO_ENCODING, so we don't here.
+			 */
 			return v4l2_ctrl_query_fill(qctrl,
 					V4L2_MPEG_AUDIO_ENCODING_LAYER_2,
 					V4L2_MPEG_AUDIO_ENCODING_AC3, 1,
 					default_params.audio_encoding);
+		}
 
 		return v4l2_ctrl_query_fill(qctrl,
 				V4L2_MPEG_AUDIO_ENCODING_LAYER_2,
 				V4L2_MPEG_AUDIO_ENCODING_LAYER_2, 1,
 				default_params.audio_encoding);
 
 	case V4L2_CID_MPEG_AUDIO_L2_BITRATE:
-		return v4l2_ctrl_query_fill(qctrl,
+		err = v4l2_ctrl_query_fill(qctrl,
 				V4L2_MPEG_AUDIO_L2_BITRATE_192K,
 				V4L2_MPEG_AUDIO_L2_BITRATE_384K, 1,
 				default_params.audio_l2_bitrate);
+		if (err)
+			return err;
+		if (params->capabilities & CX2341X_CAP_HAS_AC3 &&
+		    params->audio_encoding != V4L2_MPEG_AUDIO_ENCODING_LAYER_2)
+			qctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
+		return 0;
 
 	case V4L2_CID_MPEG_AUDIO_L1_BITRATE:
 	case V4L2_CID_MPEG_AUDIO_L3_BITRATE:
 		return -EINVAL;
 
 	case V4L2_CID_MPEG_AUDIO_AC3_BITRATE:
-		return v4l2_ctrl_query_fill(qctrl,
+		err = v4l2_ctrl_query_fill(qctrl,
 				V4L2_MPEG_AUDIO_AC3_BITRATE_48K,
 				V4L2_MPEG_AUDIO_AC3_BITRATE_448K, 1,
 				default_params.audio_ac3_bitrate);
+		if (err)
+			return err;
+		if (params->capabilities & CX2341X_CAP_HAS_AC3) {
+			if (params->audio_encoding !=
+						   V4L2_MPEG_AUDIO_ENCODING_AC3)
+				qctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
+		} else
+			qctrl->flags |= V4L2_CTRL_FLAG_DISABLED;
+		return 0;
 
 	case V4L2_CID_MPEG_AUDIO_MODE_EXTENSION:
 		err = v4l2_ctrl_query_fill_std(qctrl);
@@ -771,9 +795,9 @@ const char **cx2341x_ctrl_get_menu(const struct cx2341x_mpeg_params *p, u32 id)
 EXPORT_SYMBOL(cx2341x_ctrl_get_menu);
 
 /* definitions for audio properties bits 29-28 */
-#define CX2341X_AUDIO_ENCDING_METHOD_MPEG	0
-#define CX2341X_AUDIO_ENCDING_METHOD_AC3	1
-#define CX2341X_AUDIO_ENCDING_METHOD_LPCM	2
+#define CX2341X_AUDIO_ENCODING_METHOD_MPEG	0
+#define CX2341X_AUDIO_ENCODING_METHOD_AC3	1
+#define CX2341X_AUDIO_ENCODING_METHOD_LPCM	2
 
 static void cx2341x_calc_audio_properties(struct cx2341x_mpeg_params *params)
 {
@@ -791,7 +815,7 @@ static void cx2341x_calc_audio_properties(struct cx2341x_mpeg_params *params)
 			/* Not sure if this MPEG Layer II setting is required */
 			((3 - V4L2_MPEG_AUDIO_ENCODING_LAYER_2) << 2) |
 			(params->audio_ac3_bitrate << 4) |
-			(CX2341X_AUDIO_ENCDING_METHOD_AC3 << 28);
+			(CX2341X_AUDIO_ENCODING_METHOD_AC3 << 28);
 	} else {
 		/* Assuming MPEG Layer II */
 		params->audio_properties |=

diff --git a/include/media/cx2341x.h b/include/media/cx2341x.h
@@ -29,7 +29,6 @@ enum cx2341x_cap {
 	CX2341X_CAP_HAS_SLICED_VBI = 1 << 0,
 	CX2341X_CAP_HAS_TS 	   = 1 << 1,
 	CX2341X_CAP_HAS_AC3 	   = 1 << 2,
-	CX2341X_CAP_HAS_LPCM	   = 1 << 3,
 };
 
 struct cx2341x_mpeg_params {