Mailing List Archive

Andy,

Thank you so much for the fast response.

I know that there are some firmware issues which we need to sort out.
The AC3 firmware, needs to be updated in Cx18 firmware.

As soon as we resolve this on firmware side, I will let you know with details.

Thanks
Sri

-----Original Message-----
From: Andy Walls [mailto:awalls@radix.net]
Sent: Thursday, January 01, 2009 2:20 PM
To: ivtv-devel@ivtvdriver.org; video4linux-list@redhat.com
Cc: Hans Verkuil
Subject: [PATCH] cx18, cx2341x, ivtv: Add AC-3 audio encoding control to cx18

The patch in line below adds a control to the cx18 driver to request
AC-3 audio instead of MPEG Layer II. It doesn't quite work yet due to
cx18 firmware issues.

However, I think I've got the basic control work done and need a review
to make sure I didn't muck anything up with the cx2341x or ivtv modules.

Of particular concern to me is

a) changing the cx2341x "audio_properties" from a u16 to a u32, as this
is what rippled down in source code to the to ivtv driver.

b) accidentally adding a bogus options or controls to ivtv.


The change can also be found at

http://linuxtv.org/hg/~awalls/v4l-dvb

Regards,
Andy


# HG changeset patch
# User Andy Walls <awalls@radix.net>
# Date 1230847351 18000
# Node ID e9cf344a6749de5d3778fac3c7114476f7f0b647
# Parent 41242777b3d8bb162c65c2d0b2c542417b72d946
cx18, cx2341x, ivtv: Add AC-3 audio encoding control to cx18

From: Andy Walls <awalls@radix.net>

Initial addition of controls to set AC-3 audio encoding for the CX23418 - it
does not work yet due to firmware or cx18 driver issues. This change affects
the common cx2341x and ivtv modules due to shared structures and
common functions.

Priority: normal

Signed-off-by: Andy Walls <awalls@radix.net>

diff -r 41242777b3d8 -r e9cf344a6749 linux/drivers/media/video/cx18/cx18-driver.c
--- a/linux/drivers/media/video/cx18/cx18-driver.c Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/cx18/cx18-driver.c Thu Jan 01 17:02:31 2009 -0500
@@ -592,7 +592,8 @@ static int __devinit cx18_init_struct1(s
(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;
+ cx->params.capabilities = CX2341X_CAP_HAS_TS | CX2341X_CAP_HAS_AC3 |
+ CX2341X_CAP_HAS_LPCM;
init_waitqueue_head(&cx->cap_w);
init_waitqueue_head(&cx->mb_apu_waitq);
init_waitqueue_head(&cx->mb_cpu_waitq);
diff -r 41242777b3d8 -r e9cf344a6749 linux/drivers/media/video/cx18/cx18-driver.h
--- a/linux/drivers/media/video/cx18/cx18-driver.h Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/cx18/cx18-driver.h Thu Jan 01 17:02:31 2009 -0500
@@ -413,7 +413,7 @@ struct cx18 {

/* dualwatch */
unsigned long dualwatch_jiffies;
- u16 dualwatch_stereo_mode;
+ u32 dualwatch_stereo_mode;

/* Digitizer type */
int digitizer; /* 0x00EF = saa7114 0x00FO = saa7115 0x0106 = mic */
diff -r 41242777b3d8 -r e9cf344a6749 linux/drivers/media/video/cx18/cx18-fileops.c
--- a/linux/drivers/media/video/cx18/cx18-fileops.c Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/cx18/cx18-fileops.c Thu Jan 01 17:02:31 2009 -0500
@@ -128,10 +128,10 @@ static void cx18_dualwatch(struct cx18 *
static void cx18_dualwatch(struct cx18 *cx)
{
struct v4l2_tuner vt;
- u16 new_bitmap;
- u16 new_stereo_mode;
- const u16 stereo_mask = 0x0300;
- const u16 dual = 0x0200;
+ u32 new_bitmap;
+ u32 new_stereo_mode;
+ const u32 stereo_mask = 0x0300;
+ const u32 dual = 0x0200;
u32 h;

new_stereo_mode = cx->params.audio_properties & stereo_mask;
diff -r 41242777b3d8 -r e9cf344a6749 linux/drivers/media/video/cx2341x.c
--- a/linux/drivers/media/video/cx2341x.c Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/cx2341x.c Thu Jan 01 17:02:31 2009 -0500
@@ -1,5 +1,5 @@
/*
- * cx2341x - generic code for cx23415/6 based devices
+ * cx2341x - generic code for cx23415/6/8 based devices
*
* Copyright (C) 2006 Hans Verkuil <hverkuil@xs4all.nl>
*
@@ -31,7 +31,7 @@
#include <media/v4l2-common.h>
#include "compat.h"

-MODULE_DESCRIPTION("cx23415/6 driver");
+MODULE_DESCRIPTION("cx23415/6/8 driver");
MODULE_AUTHOR("Hans Verkuil");
MODULE_LICENSE("GPL");

@@ -46,6 +46,7 @@ const u32 cx2341x_mpeg_ctrls[] = {
V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ,
V4L2_CID_MPEG_AUDIO_ENCODING,
V4L2_CID_MPEG_AUDIO_L2_BITRATE,
+ V4L2_CID_MPEG_AUDIO_AC3_BITRATE,
V4L2_CID_MPEG_AUDIO_MODE,
V4L2_CID_MPEG_AUDIO_MODE_EXTENSION,
V4L2_CID_MPEG_AUDIO_EMPHASIS,
@@ -95,6 +96,7 @@ static const struct cx2341x_mpeg_params
.audio_sampling_freq = V4L2_MPEG_AUDIO_SAMPLING_FREQ_48000,
.audio_encoding = V4L2_MPEG_AUDIO_ENCODING_LAYER_2,
.audio_l2_bitrate = V4L2_MPEG_AUDIO_L2_BITRATE_224K,
+ .audio_ac3_bitrate = V4L2_MPEG_AUDIO_AC3_BITRATE_224K,
.audio_mode = V4L2_MPEG_AUDIO_MODE_STEREO,
.audio_mode_extension = V4L2_MPEG_AUDIO_MODE_EXTENSION_BOUND_4,
.audio_emphasis = V4L2_MPEG_AUDIO_EMPHASIS_NONE,
@@ -149,6 +151,9 @@ static int cx2341x_get_ctrl(const struct
case V4L2_CID_MPEG_AUDIO_L2_BITRATE:
ctrl->value = params->audio_l2_bitrate;
break;
+ case V4L2_CID_MPEG_AUDIO_AC3_BITRATE:
+ ctrl->value = params->audio_ac3_bitrate;
+ break;
case V4L2_CID_MPEG_AUDIO_MODE:
ctrl->value = params->audio_mode;
break;
@@ -257,12 +262,23 @@ static int cx2341x_set_ctrl(struct cx234
params->audio_sampling_freq = ctrl->value;
break;
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;
params->audio_encoding = ctrl->value;
break;
case V4L2_CID_MPEG_AUDIO_L2_BITRATE:
if (busy)
return -EBUSY;
params->audio_l2_bitrate = ctrl->value;
+ break;
+ case V4L2_CID_MPEG_AUDIO_AC3_BITRATE:
+ if (busy)
+ return -EBUSY;
+ params->audio_ac3_bitrate = ctrl->value;
break;
case V4L2_CID_MPEG_AUDIO_MODE:
params->audio_mode = ctrl->value;
@@ -483,6 +499,12 @@ int cx2341x_ctrl_query(const struct cx23

switch (qctrl->id) {
case V4L2_CID_MPEG_AUDIO_ENCODING:
+ if (params->capabilities & CX2341X_CAP_HAS_AC3)
+ 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,
@@ -497,6 +519,12 @@ int cx2341x_ctrl_query(const struct cx23
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,
+ V4L2_MPEG_AUDIO_AC3_BITRATE_48K,
+ V4L2_MPEG_AUDIO_AC3_BITRATE_448K, 1,
+ default_params.audio_ac3_bitrate);

case V4L2_CID_MPEG_AUDIO_MODE_EXTENSION:
err = v4l2_ctrl_query_fill_std(qctrl);
@@ -672,6 +700,15 @@ const char **cx2341x_ctrl_get_menu(const
NULL
};

+ static const char *mpeg_audio_encoding_l2_ac3[] = {
+ "",
+ "MPEG-1/2 Layer II",
+ "",
+ "",
+ "AC-3",
+ NULL
+ };
+
static const char *cx2341x_video_spatial_filter_mode_menu[] = {
"Manual",
"Auto",
@@ -712,6 +749,9 @@ const char **cx2341x_ctrl_get_menu(const
case V4L2_CID_MPEG_STREAM_TYPE:
return (p->capabilities & CX2341X_CAP_HAS_TS) ?
mpeg_stream_type_with_ts : mpeg_stream_type_without_ts;
+ case V4L2_CID_MPEG_AUDIO_ENCODING:
+ return (p->capabilities & CX2341X_CAP_HAS_AC3) ?
+ mpeg_audio_encoding_l2_ac3 : v4l2_ctrl_get_menu(id);
case V4L2_CID_MPEG_AUDIO_L1_BITRATE:
case V4L2_CID_MPEG_AUDIO_L3_BITRATE:
return NULL;
@@ -731,16 +771,36 @@ const char **cx2341x_ctrl_get_menu(const
}
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
+
static void cx2341x_calc_audio_properties(struct cx2341x_mpeg_params *params)
{
- params->audio_properties = (params->audio_sampling_freq << 0) |
- ((3 - params->audio_encoding) << 2) |
- ((1 + params->audio_l2_bitrate) << 4) |
+ params->audio_properties =
+ (params->audio_sampling_freq << 0) |
(params->audio_mode << 8) |
(params->audio_mode_extension << 10) |
(((params->audio_emphasis == V4L2_MPEG_AUDIO_EMPHASIS_CCITT_J17)
? 3 : params->audio_emphasis) << 12) |
(params->audio_crc << 14);
+
+ if ((params->capabilities & CX2341X_CAP_HAS_AC3) &&
+ params->audio_encoding == V4L2_MPEG_AUDIO_ENCODING_AC3) {
+ params->audio_properties |=
+#if 1
+ /* Not sure if this MPEG Layer II setting is required */
+ ((3 - V4L2_MPEG_AUDIO_ENCODING_LAYER_2) << 2) |
+#endif
+ (params->audio_ac3_bitrate << 4) |
+ (CX2341X_AUDIO_ENCDING_METHOD_AC3 << 28);
+ } else {
+ /* Assuming MPEG Layer II */
+ params->audio_properties |=
+ ((3 - params->audio_encoding) << 2) |
+ ((1 + params->audio_l2_bitrate) << 4);
+ }
}

int cx2341x_ext_ctrls(struct cx2341x_mpeg_params *params, int busy,
@@ -1023,7 +1083,10 @@ void cx2341x_log_status(const struct cx2
prefix,
cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ),
cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_ENCODING),
- cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_L2_BITRATE),
+ cx2341x_menu_item(p,
+ p->audio_encoding == V4L2_MPEG_AUDIO_ENCODING_AC3
+ ? V4L2_CID_MPEG_AUDIO_AC3_BITRATE
+ : V4L2_CID_MPEG_AUDIO_L2_BITRATE),
cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_MODE),
p->audio_mute ? " (muted)" : "");
if (p->audio_mode == V4L2_MPEG_AUDIO_MODE_JOINT_STEREO)
diff -r 41242777b3d8 -r e9cf344a6749 linux/drivers/media/video/ivtv/ivtv-driver.h
--- a/linux/drivers/media/video/ivtv/ivtv-driver.h Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/ivtv/ivtv-driver.h Thu Jan 01 17:02:31 2009 -0500
@@ -697,7 +697,7 @@ struct ivtv {
u64 vbi_data_inserted; /* number of VBI bytes inserted into the MPEG stream */
u32 last_dec_timing[3]; /* cache last retrieved pts/scr/frame values */
unsigned long dualwatch_jiffies;/* jiffies value of the previous dualwatch check */
- u16 dualwatch_stereo_mode; /* current detected dualwatch stereo mode */
+ u32 dualwatch_stereo_mode; /* current detected dualwatch stereo mode */


/* VBI state info */
diff -r 41242777b3d8 -r e9cf344a6749 linux/drivers/media/video/ivtv/ivtv-fileops.c
--- a/linux/drivers/media/video/ivtv/ivtv-fileops.c Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/ivtv/ivtv-fileops.c Thu Jan 01 17:02:31 2009 -0500
@@ -148,10 +148,10 @@ static void ivtv_dualwatch(struct ivtv *
static void ivtv_dualwatch(struct ivtv *itv)
{
struct v4l2_tuner vt;
- u16 new_bitmap;
- u16 new_stereo_mode;
- const u16 stereo_mask = 0x0300;
- const u16 dual = 0x0200;
+ u32 new_bitmap;
+ u32 new_stereo_mode;
+ const u32 stereo_mask = 0x0300;
+ const u32 dual = 0x0200;

new_stereo_mode = itv->params.audio_properties & stereo_mask;
memset(&vt, 0, sizeof(vt));
diff -r 41242777b3d8 -r e9cf344a6749 linux/include/media/cx2341x.h
--- a/linux/include/media/cx2341x.h Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/include/media/cx2341x.h Thu Jan 01 17:02:31 2009 -0500
@@ -1,5 +1,5 @@
/*
- cx23415/6 header containing common defines.
+ cx23415/6/8 header containing common defines.

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -28,6 +28,8 @@ enum cx2341x_cap {
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 {
@@ -47,11 +49,12 @@ struct cx2341x_mpeg_params {
enum v4l2_mpeg_audio_sampling_freq audio_sampling_freq;
enum v4l2_mpeg_audio_encoding audio_encoding;
enum v4l2_mpeg_audio_l2_bitrate audio_l2_bitrate;
+ enum v4l2_mpeg_audio_ac3_bitrate audio_ac3_bitrate;
enum v4l2_mpeg_audio_mode audio_mode;
enum v4l2_mpeg_audio_mode_extension audio_mode_extension;
enum v4l2_mpeg_audio_emphasis audio_emphasis;
enum v4l2_mpeg_audio_crc audio_crc;
- u16 audio_properties;
+ u32 audio_properties;
u16 audio_mute;

/* video */



Conexant E-mail Firewall (Conexant.Com) made the following annotations
---------------------------------------------------------------------
********************** Legal Disclaimer ****************************

"This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you."

**********************************************************************

---------------------------------------------------------------------


_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

Hi Andy,

I've done a quick review:

On Thursday 01 January 2009 23:20:08 Andy Walls wrote:
> The patch in line below adds a control to the cx18 driver to request
> AC-3 audio instead of MPEG Layer II. It doesn't quite work yet due to
> cx18 firmware issues.
>
> However, I think I've got the basic control work done and need a review
> to make sure I didn't muck anything up with the cx2341x or ivtv modules.
>
> Of particular concern to me is
>
> a) changing the cx2341x "audio_properties" from a u16 to a u32, as this
> is what rippled down in source code to the to ivtv driver.
>
> b) accidentally adding a bogus options or controls to ivtv.
>
>
> The change can also be found at
>
> http://linuxtv.org/hg/~awalls/v4l-dvb
>
> Regards,
> Andy
>
>
> # HG changeset patch
> # User Andy Walls <awalls@radix.net>
> # Date 1230847351 18000
> # Node ID e9cf344a6749de5d3778fac3c7114476f7f0b647
> # Parent 41242777b3d8bb162c65c2d0b2c542417b72d946
> cx18, cx2341x, ivtv: Add AC-3 audio encoding control to cx18
>
> From: Andy Walls <awalls@radix.net>
>
> Initial addition of controls to set AC-3 audio encoding for the CX23418 -
> it does not work yet due to firmware or cx18 driver issues. This change
> affects the common cx2341x and ivtv modules due to shared structures and
> common functions.
>
> Priority: normal
>
> Signed-off-by: Andy Walls <awalls@radix.net>
>
> diff -r 41242777b3d8 -r e9cf344a6749
> linux/drivers/media/video/cx18/cx18-driver.c ---
> a/linux/drivers/media/video/cx18/cx18-driver.c Thu Jan 01 10:35:06 2009
> -0500 +++ b/linux/drivers/media/video/cx18/cx18-driver.c Thu Jan 01
> 17:02:31 2009 -0500 @@ -592,7 +592,8 @@ static int __devinit
> cx18_init_struct1(s
> (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;
> + cx->params.capabilities = CX2341X_CAP_HAS_TS | CX2341X_CAP_HAS_AC3 |
> + CX2341X_CAP_HAS_LPCM;
> init_waitqueue_head(&cx->cap_w);
> init_waitqueue_head(&cx->mb_apu_waitq);
> init_waitqueue_head(&cx->mb_cpu_waitq);
> diff -r 41242777b3d8 -r e9cf344a6749
> linux/drivers/media/video/cx18/cx18-driver.h ---
> a/linux/drivers/media/video/cx18/cx18-driver.h Thu Jan 01 10:35:06 2009
> -0500 +++ b/linux/drivers/media/video/cx18/cx18-driver.h Thu Jan 01
> 17:02:31 2009 -0500 @@ -413,7 +413,7 @@ struct cx18 {
>
> /* dualwatch */
> unsigned long dualwatch_jiffies;
> - u16 dualwatch_stereo_mode;
> + u32 dualwatch_stereo_mode;
>
> /* Digitizer type */
> int digitizer; /* 0x00EF = saa7114 0x00FO = saa7115 0x0106 = mic */
> diff -r 41242777b3d8 -r e9cf344a6749
> linux/drivers/media/video/cx18/cx18-fileops.c ---
> a/linux/drivers/media/video/cx18/cx18-fileops.c Thu Jan 01 10:35:06 2009
> -0500 +++ b/linux/drivers/media/video/cx18/cx18-fileops.c Thu Jan 01
> 17:02:31 2009 -0500 @@ -128,10 +128,10 @@ static void
> cx18_dualwatch(struct cx18 *
> static void cx18_dualwatch(struct cx18 *cx)
> {
> struct v4l2_tuner vt;
> - u16 new_bitmap;
> - u16 new_stereo_mode;
> - const u16 stereo_mask = 0x0300;
> - const u16 dual = 0x0200;
> + u32 new_bitmap;
> + u32 new_stereo_mode;
> + const u32 stereo_mask = 0x0300;
> + const u32 dual = 0x0200;
> u32 h;
>
> new_stereo_mode = cx->params.audio_properties & stereo_mask;
> diff -r 41242777b3d8 -r e9cf344a6749 linux/drivers/media/video/cx2341x.c
> --- a/linux/drivers/media/video/cx2341x.c Thu Jan 01 10:35:06 2009 -0500
> +++ b/linux/drivers/media/video/cx2341x.c Thu Jan 01 17:02:31 2009 -0500
> @@ -1,5 +1,5 @@
> /*
> - * cx2341x - generic code for cx23415/6 based devices
> + * cx2341x - generic code for cx23415/6/8 based devices
> *
> * Copyright (C) 2006 Hans Verkuil <hverkuil@xs4all.nl>
> *
> @@ -31,7 +31,7 @@
> #include <media/v4l2-common.h>
> #include "compat.h"
>
> -MODULE_DESCRIPTION("cx23415/6 driver");
> +MODULE_DESCRIPTION("cx23415/6/8 driver");
> MODULE_AUTHOR("Hans Verkuil");
> MODULE_LICENSE("GPL");
>
> @@ -46,6 +46,7 @@ const u32 cx2341x_mpeg_ctrls[] = {
> V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ,
> V4L2_CID_MPEG_AUDIO_ENCODING,
> V4L2_CID_MPEG_AUDIO_L2_BITRATE,
> + V4L2_CID_MPEG_AUDIO_AC3_BITRATE,
> V4L2_CID_MPEG_AUDIO_MODE,
> V4L2_CID_MPEG_AUDIO_MODE_EXTENSION,
> V4L2_CID_MPEG_AUDIO_EMPHASIS,
> @@ -95,6 +96,7 @@ static const struct cx2341x_mpeg_params
> .audio_sampling_freq = V4L2_MPEG_AUDIO_SAMPLING_FREQ_48000,
> .audio_encoding = V4L2_MPEG_AUDIO_ENCODING_LAYER_2,
> .audio_l2_bitrate = V4L2_MPEG_AUDIO_L2_BITRATE_224K,
> + .audio_ac3_bitrate = V4L2_MPEG_AUDIO_AC3_BITRATE_224K,
> .audio_mode = V4L2_MPEG_AUDIO_MODE_STEREO,
> .audio_mode_extension = V4L2_MPEG_AUDIO_MODE_EXTENSION_BOUND_4,
> .audio_emphasis = V4L2_MPEG_AUDIO_EMPHASIS_NONE,
> @@ -149,6 +151,9 @@ static int cx2341x_get_ctrl(const struct
> case V4L2_CID_MPEG_AUDIO_L2_BITRATE:
> ctrl->value = params->audio_l2_bitrate;
> break;
> + case V4L2_CID_MPEG_AUDIO_AC3_BITRATE:
> + ctrl->value = params->audio_ac3_bitrate;
> + break;
> case V4L2_CID_MPEG_AUDIO_MODE:
> ctrl->value = params->audio_mode;
> break;
> @@ -257,12 +262,23 @@ static int cx2341x_set_ctrl(struct cx234
> params->audio_sampling_freq = ctrl->value;
> break;
> 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;

This can't be right: if CAP_HAS_AC3 is not set, then it will always
return -EINVAL.

> params->audio_encoding = ctrl->value;
> break;
> case V4L2_CID_MPEG_AUDIO_L2_BITRATE:
> if (busy)
> return -EBUSY;
> params->audio_l2_bitrate = ctrl->value;
> + break;
> + case V4L2_CID_MPEG_AUDIO_AC3_BITRATE:
> + if (busy)
> + return -EBUSY;
> + params->audio_ac3_bitrate = ctrl->value;
> break;

This should test for CAP_HAS_AC3 as well.

> case V4L2_CID_MPEG_AUDIO_MODE:
> params->audio_mode = ctrl->value;
> @@ -483,6 +499,12 @@ int cx2341x_ctrl_query(const struct cx23
>
> switch (qctrl->id) {
> case V4L2_CID_MPEG_AUDIO_ENCODING:
> + if (params->capabilities & CX2341X_CAP_HAS_AC3)
> + 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,
> @@ -497,6 +519,12 @@ int cx2341x_ctrl_query(const struct cx23
> 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,
> + V4L2_MPEG_AUDIO_AC3_BITRATE_48K,
> + V4L2_MPEG_AUDIO_AC3_BITRATE_448K, 1,
> + default_params.audio_ac3_bitrate);

Also needs a test.

> case V4L2_CID_MPEG_AUDIO_MODE_EXTENSION:
> err = v4l2_ctrl_query_fill_std(qctrl);
> @@ -672,6 +700,15 @@ const char **cx2341x_ctrl_get_menu(const
> NULL
> };
>
> + static const char *mpeg_audio_encoding_l2_ac3[] = {
> + "",
> + "MPEG-1/2 Layer II",
> + "",
> + "",
> + "AC-3",
> + NULL
> + };
> +
> static const char *cx2341x_video_spatial_filter_mode_menu[] = {
> "Manual",
> "Auto",
> @@ -712,6 +749,9 @@ const char **cx2341x_ctrl_get_menu(const
> case V4L2_CID_MPEG_STREAM_TYPE:
> return (p->capabilities & CX2341X_CAP_HAS_TS) ?
> mpeg_stream_type_with_ts : mpeg_stream_type_without_ts;
> + case V4L2_CID_MPEG_AUDIO_ENCODING:
> + return (p->capabilities & CX2341X_CAP_HAS_AC3) ?
> + mpeg_audio_encoding_l2_ac3 : v4l2_ctrl_get_menu(id);
> case V4L2_CID_MPEG_AUDIO_L1_BITRATE:
> case V4L2_CID_MPEG_AUDIO_L3_BITRATE:
> return NULL;
> @@ -731,16 +771,36 @@ const char **cx2341x_ctrl_get_menu(const
> }
> 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

ENCDING? You mean ENCODING.

> +
> static void cx2341x_calc_audio_properties(struct cx2341x_mpeg_params
> *params) {
> - params->audio_properties = (params->audio_sampling_freq << 0) |
> - ((3 - params->audio_encoding) << 2) |
> - ((1 + params->audio_l2_bitrate) << 4) |
> + params->audio_properties =
> + (params->audio_sampling_freq << 0) |
> (params->audio_mode << 8) |
> (params->audio_mode_extension << 10) |
> (((params->audio_emphasis == V4L2_MPEG_AUDIO_EMPHASIS_CCITT_J17)
> ? 3 : params->audio_emphasis) << 12) |
> (params->audio_crc << 14);
> +
> + if ((params->capabilities & CX2341X_CAP_HAS_AC3) &&
> + params->audio_encoding == V4L2_MPEG_AUDIO_ENCODING_AC3) {
> + params->audio_properties |=
> +#if 1
> + /* Not sure if this MPEG Layer II setting is required */
> + ((3 - V4L2_MPEG_AUDIO_ENCODING_LAYER_2) << 2) |
> +#endif
> + (params->audio_ac3_bitrate << 4) |
> + (CX2341X_AUDIO_ENCDING_METHOD_AC3 << 28);
> + } else {
> + /* Assuming MPEG Layer II */
> + params->audio_properties |=
> + ((3 - params->audio_encoding) << 2) |
> + ((1 + params->audio_l2_bitrate) << 4);
> + }
> }
>
> int cx2341x_ext_ctrls(struct cx2341x_mpeg_params *params, int busy,
> @@ -1023,7 +1083,10 @@ void cx2341x_log_status(const struct cx2
> prefix,
> cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ),
> cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_ENCODING),
> - cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_L2_BITRATE),
> + cx2341x_menu_item(p,
> + p->audio_encoding == V4L2_MPEG_AUDIO_ENCODING_AC3
> + ? V4L2_CID_MPEG_AUDIO_AC3_BITRATE
> + : V4L2_CID_MPEG_AUDIO_L2_BITRATE),
> cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_MODE),
> p->audio_mute ? " (muted)" : "");
> if (p->audio_mode == V4L2_MPEG_AUDIO_MODE_JOINT_STEREO)
> diff -r 41242777b3d8 -r e9cf344a6749
> linux/drivers/media/video/ivtv/ivtv-driver.h ---
> a/linux/drivers/media/video/ivtv/ivtv-driver.h Thu Jan 01 10:35:06 2009
> -0500 +++ b/linux/drivers/media/video/ivtv/ivtv-driver.h Thu Jan 01
> 17:02:31 2009 -0500 @@ -697,7 +697,7 @@ struct ivtv {
> u64 vbi_data_inserted; /* number of VBI bytes inserted into
> the MPEG stream */ u32 last_dec_timing[3]; /* cache last
> retrieved pts/scr/frame values */ unsigned long dualwatch_jiffies;/*
> jiffies value of the previous dualwatch check */ - u16
> dualwatch_stereo_mode; /* current detected dualwatch stereo mode */
> + u32 dualwatch_stereo_mode; /* current detected dualwatch stereo
> mode */
>
>
> /* VBI state info */
> diff -r 41242777b3d8 -r e9cf344a6749
> linux/drivers/media/video/ivtv/ivtv-fileops.c ---
> a/linux/drivers/media/video/ivtv/ivtv-fileops.c Thu Jan 01 10:35:06 2009
> -0500 +++ b/linux/drivers/media/video/ivtv/ivtv-fileops.c Thu Jan 01
> 17:02:31 2009 -0500 @@ -148,10 +148,10 @@ static void
> ivtv_dualwatch(struct ivtv *
> static void ivtv_dualwatch(struct ivtv *itv)
> {
> struct v4l2_tuner vt;
> - u16 new_bitmap;
> - u16 new_stereo_mode;
> - const u16 stereo_mask = 0x0300;
> - const u16 dual = 0x0200;
> + u32 new_bitmap;
> + u32 new_stereo_mode;
> + const u32 stereo_mask = 0x0300;
> + const u32 dual = 0x0200;
>
> new_stereo_mode = itv->params.audio_properties & stereo_mask;
> memset(&vt, 0, sizeof(vt));
> diff -r 41242777b3d8 -r e9cf344a6749 linux/include/media/cx2341x.h
> --- a/linux/include/media/cx2341x.h Thu Jan 01 10:35:06 2009 -0500
> +++ b/linux/include/media/cx2341x.h Thu Jan 01 17:02:31 2009 -0500
> @@ -1,5 +1,5 @@
> /*
> - cx23415/6 header containing common defines.
> + cx23415/6/8 header containing common defines.
>
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> @@ -28,6 +28,8 @@ enum cx2341x_cap {
> 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 {
> @@ -47,11 +49,12 @@ struct cx2341x_mpeg_params {
> enum v4l2_mpeg_audio_sampling_freq audio_sampling_freq;
> enum v4l2_mpeg_audio_encoding audio_encoding;
> enum v4l2_mpeg_audio_l2_bitrate audio_l2_bitrate;
> + enum v4l2_mpeg_audio_ac3_bitrate audio_ac3_bitrate;
> enum v4l2_mpeg_audio_mode audio_mode;
> enum v4l2_mpeg_audio_mode_extension audio_mode_extension;
> enum v4l2_mpeg_audio_emphasis audio_emphasis;
> enum v4l2_mpeg_audio_crc audio_crc;
> - u16 audio_properties;
> + u32 audio_properties;
> u16 audio_mute;
>
> /* video */

Regards,

Hans

--
Hans Verkuil - video4linux developer - sponsored by TANDBERG

_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

On Sat, 2009-01-03 at 10:36 +0100, Hans Verkuil wrote:
> Hi Andy,
>
> I've done a quick review:
>
> On Thursday 01 January 2009 23:20:08 Andy Walls wrote:
> > The patch in line below adds a control to the cx18 driver to request
> > AC-3 audio instead of MPEG Layer II. It doesn't quite work yet due to
> > cx18 firmware issues.
> >
> > However, I think I've got the basic control work done and need a review
> > to make sure I didn't muck anything up with the cx2341x or ivtv modules.
> >
> > Of particular concern to me is
> >
> > a) changing the cx2341x "audio_properties" from a u16 to a u32, as this
> > is what rippled down in source code to the to ivtv driver.
> >
> > b) accidentally adding a bogus options or controls to ivtv.
> >
> >
> > The change can also be found at
> >
> > http://linuxtv.org/hg/~awalls/v4l-dvb
> >
> > Regards,
> > Andy
> >
> >
> > # HG changeset patch
> > # User Andy Walls <awalls@radix.net>
> > # Date 1230847351 18000
> > # Node ID e9cf344a6749de5d3778fac3c7114476f7f0b647
> > # Parent 41242777b3d8bb162c65c2d0b2c542417b72d946
> > cx18, cx2341x, ivtv: Add AC-3 audio encoding control to cx18
> >
> > From: Andy Walls <awalls@radix.net>
> >
> > Initial addition of controls to set AC-3 audio encoding for the CX23418 -
> > it does not work yet due to firmware or cx18 driver issues. This change
> > affects the common cx2341x and ivtv modules due to shared structures and
> > common functions.
> >
> > Priority: normal
> >
> > Signed-off-by: Andy Walls <awalls@radix.net>
> >
> > diff -r 41242777b3d8 -r e9cf344a6749
> > linux/drivers/media/video/cx18/cx18-driver.c ---
> > a/linux/drivers/media/video/cx18/cx18-driver.c Thu Jan 01 10:35:06 2009
> > -0500 +++ b/linux/drivers/media/video/cx18/cx18-driver.c Thu Jan 01
> > 17:02:31 2009 -0500 @@ -592,7 +592,8 @@ static int __devinit
> > cx18_init_struct1(s
> > (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;
> > + cx->params.capabilities = CX2341X_CAP_HAS_TS | CX2341X_CAP_HAS_AC3 |
> > + CX2341X_CAP_HAS_LPCM;
> > init_waitqueue_head(&cx->cap_w);
> > init_waitqueue_head(&cx->mb_apu_waitq);
> > init_waitqueue_head(&cx->mb_cpu_waitq);
> > diff -r 41242777b3d8 -r e9cf344a6749
> > linux/drivers/media/video/cx18/cx18-driver.h ---
> > a/linux/drivers/media/video/cx18/cx18-driver.h Thu Jan 01 10:35:06 2009
> > -0500 +++ b/linux/drivers/media/video/cx18/cx18-driver.h Thu Jan 01
> > 17:02:31 2009 -0500 @@ -413,7 +413,7 @@ struct cx18 {
> >
> > /* dualwatch */
> > unsigned long dualwatch_jiffies;
> > - u16 dualwatch_stereo_mode;
> > + u32 dualwatch_stereo_mode;
> >
> > /* Digitizer type */
> > int digitizer; /* 0x00EF = saa7114 0x00FO = saa7115 0x0106 = mic */
> > diff -r 41242777b3d8 -r e9cf344a6749
> > linux/drivers/media/video/cx18/cx18-fileops.c ---
> > a/linux/drivers/media/video/cx18/cx18-fileops.c Thu Jan 01 10:35:06 2009
> > -0500 +++ b/linux/drivers/media/video/cx18/cx18-fileops.c Thu Jan 01
> > 17:02:31 2009 -0500 @@ -128,10 +128,10 @@ static void
> > cx18_dualwatch(struct cx18 *
> > static void cx18_dualwatch(struct cx18 *cx)
> > {
> > struct v4l2_tuner vt;
> > - u16 new_bitmap;
> > - u16 new_stereo_mode;
> > - const u16 stereo_mask = 0x0300;
> > - const u16 dual = 0x0200;
> > + u32 new_bitmap;
> > + u32 new_stereo_mode;
> > + const u32 stereo_mask = 0x0300;
> > + const u32 dual = 0x0200;
> > u32 h;
> >
> > new_stereo_mode = cx->params.audio_properties & stereo_mask;
> > diff -r 41242777b3d8 -r e9cf344a6749 linux/drivers/media/video/cx2341x.c
> > --- a/linux/drivers/media/video/cx2341x.c Thu Jan 01 10:35:06 2009 -0500
> > +++ b/linux/drivers/media/video/cx2341x.c Thu Jan 01 17:02:31 2009 -0500
> > @@ -1,5 +1,5 @@
> > /*
> > - * cx2341x - generic code for cx23415/6 based devices
> > + * cx2341x - generic code for cx23415/6/8 based devices
> > *
> > * Copyright (C) 2006 Hans Verkuil <hverkuil@xs4all.nl>
> > *
> > @@ -31,7 +31,7 @@
> > #include <media/v4l2-common.h>
> > #include "compat.h"
> >
> > -MODULE_DESCRIPTION("cx23415/6 driver");
> > +MODULE_DESCRIPTION("cx23415/6/8 driver");
> > MODULE_AUTHOR("Hans Verkuil");
> > MODULE_LICENSE("GPL");
> >
> > @@ -46,6 +46,7 @@ const u32 cx2341x_mpeg_ctrls[] = {
> > V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ,
> > V4L2_CID_MPEG_AUDIO_ENCODING,
> > V4L2_CID_MPEG_AUDIO_L2_BITRATE,
> > + V4L2_CID_MPEG_AUDIO_AC3_BITRATE,
> > V4L2_CID_MPEG_AUDIO_MODE,
> > V4L2_CID_MPEG_AUDIO_MODE_EXTENSION,
> > V4L2_CID_MPEG_AUDIO_EMPHASIS,
> > @@ -95,6 +96,7 @@ static const struct cx2341x_mpeg_params
> > .audio_sampling_freq = V4L2_MPEG_AUDIO_SAMPLING_FREQ_48000,
> > .audio_encoding = V4L2_MPEG_AUDIO_ENCODING_LAYER_2,
> > .audio_l2_bitrate = V4L2_MPEG_AUDIO_L2_BITRATE_224K,
> > + .audio_ac3_bitrate = V4L2_MPEG_AUDIO_AC3_BITRATE_224K,
> > .audio_mode = V4L2_MPEG_AUDIO_MODE_STEREO,
> > .audio_mode_extension = V4L2_MPEG_AUDIO_MODE_EXTENSION_BOUND_4,
> > .audio_emphasis = V4L2_MPEG_AUDIO_EMPHASIS_NONE,
> > @@ -149,6 +151,9 @@ static int cx2341x_get_ctrl(const struct
> > case V4L2_CID_MPEG_AUDIO_L2_BITRATE:
> > ctrl->value = params->audio_l2_bitrate;
> > break;
> > + case V4L2_CID_MPEG_AUDIO_AC3_BITRATE:
> > + ctrl->value = params->audio_ac3_bitrate;
> > + break;
> > case V4L2_CID_MPEG_AUDIO_MODE:
> > ctrl->value = params->audio_mode;
> > break;
> > @@ -257,12 +262,23 @@ static int cx2341x_set_ctrl(struct cx234
> > params->audio_sampling_freq = ctrl->value;
> > break;
> > 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;
>
> This can't be right: if CAP_HAS_AC3 is not set, then it will always
> return -EINVAL.

Ack! You're right. I shouldn't have done this stuff on New Year's Day
- I needed more sleep.


> > params->audio_encoding = ctrl->value;
> > break;
> > case V4L2_CID_MPEG_AUDIO_L2_BITRATE:
> > if (busy)
> > return -EBUSY;
> > params->audio_l2_bitrate = ctrl->value;
> > + break;
> > + case V4L2_CID_MPEG_AUDIO_AC3_BITRATE:
> > + if (busy)
> > + return -EBUSY;
> > + params->audio_ac3_bitrate = ctrl->value;
> > break;
>
> This should test for CAP_HAS_AC3 as well.

I wondered about this, since I added V4L2_CID_MPEG_AUDIO_AC3_BITRATE in
the cx2341x_mpeg_ctrls[] list of controls for CX2341x devices, I wasn't
sure of the user experience for always getting -EINVAL for a listed
control. There is no harm in letting the user app set a control value
that will never be used for CX23415/6 devices.

I didn't want to make an alternate cx2341x_mpeg_ctrls[] list. I'll put
in a check for the CAP_HAS_AC3 and make the control inactive or
read-only, if the CAP isn't set.


> > case V4L2_CID_MPEG_AUDIO_MODE:
> > params->audio_mode = ctrl->value;
> > @@ -483,6 +499,12 @@ int cx2341x_ctrl_query(const struct cx23
> >
> > switch (qctrl->id) {
> > case V4L2_CID_MPEG_AUDIO_ENCODING:
> > + if (params->capabilities & CX2341X_CAP_HAS_AC3)
> > + 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,
> > @@ -497,6 +519,12 @@ int cx2341x_ctrl_query(const struct cx23
> > 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,
> > + V4L2_MPEG_AUDIO_AC3_BITRATE_48K,
> > + V4L2_MPEG_AUDIO_AC3_BITRATE_448K, 1,
> > + default_params.audio_ac3_bitrate);
>
> Also needs a test.

OK. Can do.

BTW, The old code fell through to the default case. I note that the L1
& L3 bitrate controls aren't valid, are trapped here, but aren't in the
cx2341x_mpeg_ctrls[] list. I was unsure if a control not in the
cx2341x_mpeg_ctrls[] list would ever come through here.

> > case V4L2_CID_MPEG_AUDIO_MODE_EXTENSION:
> > err = v4l2_ctrl_query_fill_std(qctrl);
> > @@ -672,6 +700,15 @@ const char **cx2341x_ctrl_get_menu(const
> > NULL
> > };
> >
> > + static const char *mpeg_audio_encoding_l2_ac3[] = {
> > + "",
> > + "MPEG-1/2 Layer II",
> > + "",
> > + "",
> > + "AC-3",
> > + NULL
> > + };
> > +
> > static const char *cx2341x_video_spatial_filter_mode_menu[] = {
> > "Manual",
> > "Auto",
> > @@ -712,6 +749,9 @@ const char **cx2341x_ctrl_get_menu(const
> > case V4L2_CID_MPEG_STREAM_TYPE:
> > return (p->capabilities & CX2341X_CAP_HAS_TS) ?
> > mpeg_stream_type_with_ts : mpeg_stream_type_without_ts;
> > + case V4L2_CID_MPEG_AUDIO_ENCODING:
> > + return (p->capabilities & CX2341X_CAP_HAS_AC3) ?
> > + mpeg_audio_encoding_l2_ac3 : v4l2_ctrl_get_menu(id);
> > case V4L2_CID_MPEG_AUDIO_L1_BITRATE:
> > case V4L2_CID_MPEG_AUDIO_L3_BITRATE:
> > return NULL;
> > @@ -731,16 +771,36 @@ const char **cx2341x_ctrl_get_menu(const
> > }
> > 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
>
> ENCDING? You mean ENCODING.

Yes, thanks. Sleep deprivation and not wearing my glasses. 8-)


> > +
> > static void cx2341x_calc_audio_properties(struct cx2341x_mpeg_params
> > *params) {
> > - params->audio_properties = (params->audio_sampling_freq << 0) |
> > - ((3 - params->audio_encoding) << 2) |
> > - ((1 + params->audio_l2_bitrate) << 4) |
> > + params->audio_properties =
> > + (params->audio_sampling_freq << 0) |
> > (params->audio_mode << 8) |
> > (params->audio_mode_extension << 10) |
> > (((params->audio_emphasis == V4L2_MPEG_AUDIO_EMPHASIS_CCITT_J17)
> > ? 3 : params->audio_emphasis) << 12) |
> > (params->audio_crc << 14);
> > +
> > + if ((params->capabilities & CX2341X_CAP_HAS_AC3) &&
> > + params->audio_encoding == V4L2_MPEG_AUDIO_ENCODING_AC3) {
> > + params->audio_properties |=
> > +#if 1
> > + /* Not sure if this MPEG Layer II setting is required */
> > + ((3 - V4L2_MPEG_AUDIO_ENCODING_LAYER_2) << 2) |
> > +#endif
> > + (params->audio_ac3_bitrate << 4) |
> > + (CX2341X_AUDIO_ENCDING_METHOD_AC3 << 28);
> > + } else {
> > + /* Assuming MPEG Layer II */
> > + params->audio_properties |=
> > + ((3 - params->audio_encoding) << 2) |
> > + ((1 + params->audio_l2_bitrate) << 4);
> > + }
> > }
> >
> > int cx2341x_ext_ctrls(struct cx2341x_mpeg_params *params, int busy,
> > @@ -1023,7 +1083,10 @@ void cx2341x_log_status(const struct cx2
> > prefix,
> > cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ),
> > cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_ENCODING),
> > - cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_L2_BITRATE),
> > + cx2341x_menu_item(p,
> > + p->audio_encoding == V4L2_MPEG_AUDIO_ENCODING_AC3
> > + ? V4L2_CID_MPEG_AUDIO_AC3_BITRATE
> > + : V4L2_CID_MPEG_AUDIO_L2_BITRATE),
> > cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_MODE),
> > p->audio_mute ? " (muted)" : "");
> > if (p->audio_mode == V4L2_MPEG_AUDIO_MODE_JOINT_STEREO)
> > diff -r 41242777b3d8 -r e9cf344a6749
> > linux/drivers/media/video/ivtv/ivtv-driver.h ---
> > a/linux/drivers/media/video/ivtv/ivtv-driver.h Thu Jan 01 10:35:06 2009
> > -0500 +++ b/linux/drivers/media/video/ivtv/ivtv-driver.h Thu Jan 01
> > 17:02:31 2009 -0500 @@ -697,7 +697,7 @@ struct ivtv {
> > u64 vbi_data_inserted; /* number of VBI bytes inserted into
> > the MPEG stream */ u32 last_dec_timing[3]; /* cache last
> > retrieved pts/scr/frame values */ unsigned long dualwatch_jiffies;/*
> > jiffies value of the previous dualwatch check */ - u16
> > dualwatch_stereo_mode; /* current detected dualwatch stereo mode */
> > + u32 dualwatch_stereo_mode; /* current detected dualwatch stereo
> > mode */
> >
> >
> > /* VBI state info */
> > diff -r 41242777b3d8 -r e9cf344a6749
> > linux/drivers/media/video/ivtv/ivtv-fileops.c ---
> > a/linux/drivers/media/video/ivtv/ivtv-fileops.c Thu Jan 01 10:35:06 2009
> > -0500 +++ b/linux/drivers/media/video/ivtv/ivtv-fileops.c Thu Jan 01
> > 17:02:31 2009 -0500 @@ -148,10 +148,10 @@ static void
> > ivtv_dualwatch(struct ivtv *
> > static void ivtv_dualwatch(struct ivtv *itv)
> > {
> > struct v4l2_tuner vt;
> > - u16 new_bitmap;
> > - u16 new_stereo_mode;
> > - const u16 stereo_mask = 0x0300;
> > - const u16 dual = 0x0200;
> > + u32 new_bitmap;
> > + u32 new_stereo_mode;
> > + const u32 stereo_mask = 0x0300;
> > + const u32 dual = 0x0200;
> >
> > new_stereo_mode = itv->params.audio_properties & stereo_mask;
> > memset(&vt, 0, sizeof(vt));
> > diff -r 41242777b3d8 -r e9cf344a6749 linux/include/media/cx2341x.h
> > --- a/linux/include/media/cx2341x.h Thu Jan 01 10:35:06 2009 -0500
> > +++ b/linux/include/media/cx2341x.h Thu Jan 01 17:02:31 2009 -0500
> > @@ -1,5 +1,5 @@
> > /*
> > - cx23415/6 header containing common defines.
> > + cx23415/6/8 header containing common defines.
> >
> > This program is free software; you can redistribute it and/or modify
> > it under the terms of the GNU General Public License as published by
> > @@ -28,6 +28,8 @@ enum cx2341x_cap {
> > 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 {
> > @@ -47,11 +49,12 @@ struct cx2341x_mpeg_params {
> > enum v4l2_mpeg_audio_sampling_freq audio_sampling_freq;
> > enum v4l2_mpeg_audio_encoding audio_encoding;
> > enum v4l2_mpeg_audio_l2_bitrate audio_l2_bitrate;
> > + enum v4l2_mpeg_audio_ac3_bitrate audio_ac3_bitrate;
> > enum v4l2_mpeg_audio_mode audio_mode;
> > enum v4l2_mpeg_audio_mode_extension audio_mode_extension;
> > enum v4l2_mpeg_audio_emphasis audio_emphasis;
> > enum v4l2_mpeg_audio_crc audio_crc;
> > - u16 audio_properties;
> > + u32 audio_properties;
> > u16 audio_mute;
> >
> > /* video */
>
> Regards,
>
> Hans

Thanks for heading off that disaster of a patch.

Regards,
Andy




_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

On Sat, 2009-01-03 at 10:36 +0100, Hans Verkuil wrote:
> Hi Andy,
>
> I've done a quick review:
>
> On Thursday 01 January 2009 23:20:08 Andy Walls wrote:
> > The patch in line below adds a control to the cx18 driver to request
> > AC-3 audio instead of MPEG Layer II. It doesn't quite work yet due to
> > cx18 firmware issues.
> >
> > However, I think I've got the basic control work done and need a review
> > to make sure I didn't muck anything up with the cx2341x or ivtv modules.
> >
> > Of particular concern to me is
> >
> > a) changing the cx2341x "audio_properties" from a u16 to a u32, as this
> > is what rippled down in source code to the to ivtv driver.
> >
> > b) accidentally adding a bogus options or controls to ivtv.
> >
> >
> > The change can also be found at
> >
> > http://linuxtv.org/hg/~awalls/v4l-dvb
> >

> Regards,
>
> Hans

All,

OK. I've committed a patch to the patch in the same repo. I actually
read my (slightly dated) copy of the V4L2 API spec, and tried to bring
things into compliance with that as well. Thanks go to Hans for
pointing out my errors.

Below is the diff that includes both changes for easier review.

Regards,
Andy


diff -r 41242777b3d8 -r 70b1c2992d87 linux/drivers/media/video/cx18/cx18-driver.c
--- a/linux/drivers/media/video/cx18/cx18-driver.c Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/cx18/cx18-driver.c Sat Jan 03 12:21:30 2009 -0500
@@ -592,7 +592,7 @@ static int __devinit cx18_init_struct1(s
(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;
+ 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 -r 41242777b3d8 -r 70b1c2992d87 linux/drivers/media/video/cx18/cx18-driver.h
--- a/linux/drivers/media/video/cx18/cx18-driver.h Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/cx18/cx18-driver.h Sat Jan 03 12:21:30 2009 -0500
@@ -413,7 +413,7 @@ struct cx18 {

/* dualwatch */
unsigned long dualwatch_jiffies;
- u16 dualwatch_stereo_mode;
+ u32 dualwatch_stereo_mode;

/* Digitizer type */
int digitizer; /* 0x00EF = saa7114 0x00FO = saa7115 0x0106 = mic */
diff -r 41242777b3d8 -r 70b1c2992d87 linux/drivers/media/video/cx18/cx18-fileops.c
--- a/linux/drivers/media/video/cx18/cx18-fileops.c Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/cx18/cx18-fileops.c Sat Jan 03 12:21:30 2009 -0500
@@ -128,10 +128,10 @@ static void cx18_dualwatch(struct cx18 *
static void cx18_dualwatch(struct cx18 *cx)
{
struct v4l2_tuner vt;
- u16 new_bitmap;
- u16 new_stereo_mode;
- const u16 stereo_mask = 0x0300;
- const u16 dual = 0x0200;
+ u32 new_bitmap;
+ u32 new_stereo_mode;
+ const u32 stereo_mask = 0x0300;
+ const u32 dual = 0x0200;
u32 h;

new_stereo_mode = cx->params.audio_properties & stereo_mask;
diff -r 41242777b3d8 -r 70b1c2992d87 linux/drivers/media/video/cx2341x.c
--- a/linux/drivers/media/video/cx2341x.c Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/cx2341x.c Sat Jan 03 12:21:30 2009 -0500
@@ -1,5 +1,5 @@
/*
- * cx2341x - generic code for cx23415/6 based devices
+ * cx2341x - generic code for cx23415/6/8 based devices
*
* Copyright (C) 2006 Hans Verkuil <hverkuil@xs4all.nl>
*
@@ -31,7 +31,7 @@
#include <media/v4l2-common.h>
#include "compat.h"

-MODULE_DESCRIPTION("cx23415/6 driver");
+MODULE_DESCRIPTION("cx23415/6/8 driver");
MODULE_AUTHOR("Hans Verkuil");
MODULE_LICENSE("GPL");

@@ -46,6 +46,7 @@ const u32 cx2341x_mpeg_ctrls[] = {
V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ,
V4L2_CID_MPEG_AUDIO_ENCODING,
V4L2_CID_MPEG_AUDIO_L2_BITRATE,
+ V4L2_CID_MPEG_AUDIO_AC3_BITRATE,
V4L2_CID_MPEG_AUDIO_MODE,
V4L2_CID_MPEG_AUDIO_MODE_EXTENSION,
V4L2_CID_MPEG_AUDIO_EMPHASIS,
@@ -95,6 +96,7 @@ static const struct cx2341x_mpeg_params
.audio_sampling_freq = V4L2_MPEG_AUDIO_SAMPLING_FREQ_48000,
.audio_encoding = V4L2_MPEG_AUDIO_ENCODING_LAYER_2,
.audio_l2_bitrate = V4L2_MPEG_AUDIO_L2_BITRATE_224K,
+ .audio_ac3_bitrate = V4L2_MPEG_AUDIO_AC3_BITRATE_224K,
.audio_mode = V4L2_MPEG_AUDIO_MODE_STEREO,
.audio_mode_extension = V4L2_MPEG_AUDIO_MODE_EXTENSION_BOUND_4,
.audio_emphasis = V4L2_MPEG_AUDIO_EMPHASIS_NONE,
@@ -149,6 +151,9 @@ static int cx2341x_get_ctrl(const struct
case V4L2_CID_MPEG_AUDIO_L2_BITRATE:
ctrl->value = params->audio_l2_bitrate;
break;
+ case V4L2_CID_MPEG_AUDIO_AC3_BITRATE:
+ ctrl->value = params->audio_ac3_bitrate;
+ break;
case V4L2_CID_MPEG_AUDIO_MODE:
ctrl->value = params->audio_mode;
break;
@@ -257,12 +262,25 @@ static int cx2341x_set_ctrl(struct cx234
params->audio_sampling_freq = ctrl->value;
break;
case V4L2_CID_MPEG_AUDIO_ENCODING:
+ if (busy)
+ return -EBUSY;
+ 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:
if (busy)
return -EBUSY;
params->audio_l2_bitrate = ctrl->value;
+ break;
+ 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:
params->audio_mode = ctrl->value;
@@ -483,20 +501,54 @@ int cx2341x_ctrl_query(const struct cx23

switch (qctrl->id) {
case V4L2_CID_MPEG_AUDIO_ENCODING:
+ 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:
+ 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);
@@ -672,6 +724,15 @@ const char **cx2341x_ctrl_get_menu(const
NULL
};

+ static const char *mpeg_audio_encoding_l2_ac3[] = {
+ "",
+ "MPEG-1/2 Layer II",
+ "",
+ "",
+ "AC-3",
+ NULL
+ };
+
static const char *cx2341x_video_spatial_filter_mode_menu[] = {
"Manual",
"Auto",
@@ -712,6 +773,9 @@ const char **cx2341x_ctrl_get_menu(const
case V4L2_CID_MPEG_STREAM_TYPE:
return (p->capabilities & CX2341X_CAP_HAS_TS) ?
mpeg_stream_type_with_ts : mpeg_stream_type_without_ts;
+ case V4L2_CID_MPEG_AUDIO_ENCODING:
+ return (p->capabilities & CX2341X_CAP_HAS_AC3) ?
+ mpeg_audio_encoding_l2_ac3 : v4l2_ctrl_get_menu(id);
case V4L2_CID_MPEG_AUDIO_L1_BITRATE:
case V4L2_CID_MPEG_AUDIO_L3_BITRATE:
return NULL;
@@ -731,16 +795,36 @@ const char **cx2341x_ctrl_get_menu(const
}
EXPORT_SYMBOL(cx2341x_ctrl_get_menu);

+/* definitions for audio properties bits 29-28 */
+#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)
{
- params->audio_properties = (params->audio_sampling_freq << 0) |
- ((3 - params->audio_encoding) << 2) |
- ((1 + params->audio_l2_bitrate) << 4) |
+ params->audio_properties =
+ (params->audio_sampling_freq << 0) |
(params->audio_mode << 8) |
(params->audio_mode_extension << 10) |
(((params->audio_emphasis == V4L2_MPEG_AUDIO_EMPHASIS_CCITT_J17)
? 3 : params->audio_emphasis) << 12) |
(params->audio_crc << 14);
+
+ if ((params->capabilities & CX2341X_CAP_HAS_AC3) &&
+ params->audio_encoding == V4L2_MPEG_AUDIO_ENCODING_AC3) {
+ params->audio_properties |=
+#if 1
+ /* Not sure if this MPEG Layer II setting is required */
+ ((3 - V4L2_MPEG_AUDIO_ENCODING_LAYER_2) << 2) |
+#endif
+ (params->audio_ac3_bitrate << 4) |
+ (CX2341X_AUDIO_ENCODING_METHOD_AC3 << 28);
+ } else {
+ /* Assuming MPEG Layer II */
+ params->audio_properties |=
+ ((3 - params->audio_encoding) << 2) |
+ ((1 + params->audio_l2_bitrate) << 4);
+ }
}

int cx2341x_ext_ctrls(struct cx2341x_mpeg_params *params, int busy,
@@ -1023,7 +1107,10 @@ void cx2341x_log_status(const struct cx2
prefix,
cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ),
cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_ENCODING),
- cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_L2_BITRATE),
+ cx2341x_menu_item(p,
+ p->audio_encoding == V4L2_MPEG_AUDIO_ENCODING_AC3
+ ? V4L2_CID_MPEG_AUDIO_AC3_BITRATE
+ : V4L2_CID_MPEG_AUDIO_L2_BITRATE),
cx2341x_menu_item(p, V4L2_CID_MPEG_AUDIO_MODE),
p->audio_mute ? " (muted)" : "");
if (p->audio_mode == V4L2_MPEG_AUDIO_MODE_JOINT_STEREO)
diff -r 41242777b3d8 -r 70b1c2992d87 linux/drivers/media/video/ivtv/ivtv-driver.h
--- a/linux/drivers/media/video/ivtv/ivtv-driver.h Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/ivtv/ivtv-driver.h Sat Jan 03 12:21:30 2009 -0500
@@ -697,7 +697,7 @@ struct ivtv {
u64 vbi_data_inserted; /* number of VBI bytes inserted into the MPEG stream */
u32 last_dec_timing[3]; /* cache last retrieved pts/scr/frame values */
unsigned long dualwatch_jiffies;/* jiffies value of the previous dualwatch check */
- u16 dualwatch_stereo_mode; /* current detected dualwatch stereo mode */
+ u32 dualwatch_stereo_mode; /* current detected dualwatch stereo mode */


/* VBI state info */
diff -r 41242777b3d8 -r 70b1c2992d87 linux/drivers/media/video/ivtv/ivtv-fileops.c
--- a/linux/drivers/media/video/ivtv/ivtv-fileops.c Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/drivers/media/video/ivtv/ivtv-fileops.c Sat Jan 03 12:21:30 2009 -0500
@@ -148,10 +148,10 @@ static void ivtv_dualwatch(struct ivtv *
static void ivtv_dualwatch(struct ivtv *itv)
{
struct v4l2_tuner vt;
- u16 new_bitmap;
- u16 new_stereo_mode;
- const u16 stereo_mask = 0x0300;
- const u16 dual = 0x0200;
+ u32 new_bitmap;
+ u32 new_stereo_mode;
+ const u32 stereo_mask = 0x0300;
+ const u32 dual = 0x0200;

new_stereo_mode = itv->params.audio_properties & stereo_mask;
memset(&vt, 0, sizeof(vt));
diff -r 41242777b3d8 -r 70b1c2992d87 linux/include/media/cx2341x.h
--- a/linux/include/media/cx2341x.h Thu Jan 01 10:35:06 2009 -0500
+++ b/linux/include/media/cx2341x.h Sat Jan 03 12:21:30 2009 -0500
@@ -1,5 +1,5 @@
/*
- cx23415/6 header containing common defines.
+ cx23415/6/8 header containing common defines.

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -28,6 +28,7 @@ enum cx2341x_cap {
enum cx2341x_cap {
CX2341X_CAP_HAS_SLICED_VBI = 1 << 0,
CX2341X_CAP_HAS_TS = 1 << 1,
+ CX2341X_CAP_HAS_AC3 = 1 << 2,
};

struct cx2341x_mpeg_params {
@@ -47,11 +48,12 @@ struct cx2341x_mpeg_params {
enum v4l2_mpeg_audio_sampling_freq audio_sampling_freq;
enum v4l2_mpeg_audio_encoding audio_encoding;
enum v4l2_mpeg_audio_l2_bitrate audio_l2_bitrate;
+ enum v4l2_mpeg_audio_ac3_bitrate audio_ac3_bitrate;
enum v4l2_mpeg_audio_mode audio_mode;
enum v4l2_mpeg_audio_mode_extension audio_mode_extension;
enum v4l2_mpeg_audio_emphasis audio_emphasis;
enum v4l2_mpeg_audio_crc audio_crc;
- u16 audio_properties;
+ u32 audio_properties;
u16 audio_mute;

/* video */






_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel