cx88 default contrast,volume,balance
- From: "M. Rudowski" <mar_rud (at) poczta.onet.pl>
- Date: Fri, 10 Feb 2006 01:15:32 +0100
Dnia 2006-02-09 22:03, Uzytkownik Michael Krufky napisal:
> Ian Pickworth wrote:
>
>
>>Hello List,
>>Does no one want to adopt this poor little orphaned patch? Its a very
>>good patch :-).
>>
Yes, I also think that :)
>
> Ian-
>
> Thank you for pushing forward on this... I personally can not apply it,
> because last I knew, Mauro was working on it. I dont know why he never
> responded to your post here, but perhaps you should send it to the list
> again (after addressing my comments) and cc Mauro's infradead email.
>
> Apart from that, I have some comments inline below:
>
>
>>Ian Pickworth wrote:
>>
>>
>>>Hello List,
>>>This patch was proposed by Marcin Rudowski about a month ago, and I
>>>think it has got lost in the activity at the time. I have thus remade
>>>the patch to work on the latest CVS copy (as of "2006-01-21 04:42
>>>mcisely" in the changelog).
>>>
>>>I have been running with this patch for some time now, and appreciate
>>>the default settings being correct by default, so I think it would be
>>>a good improvement for cx88 users.
>>>
>>>I have to stress that the hard work in this was done by Marcin and
>>>Mauro.
>>>
>>>The patch signature is as follows:
>>>---------
>>> * linux/drivers/media/video/cx88/cx88-video.c: (get_control),
>>> (set_control), (init_controls), (cx88_do_ioctl):
>>> - Do some cleanups at cx88-video
>>> - Fixed a bug at balance control, when set to 0x40
>>> - improved debug messages
>>> - fixed default for contrast
>>>
>>> Signed-off-by: Ian Pickworth <ian (at) pickworth.me.uk>
>>> Signed-off-by: Marcin Rudowski <mar_rud (at) poczta.onet.pl>
>>> Signed-off-by: Mauro Carvalho Chehab <mchehab (at) brturbo.com.br>
>>>
> [snip]
>
>
>>>--- v4l-dvb/linux/drivers/media/video/cx88/cx88-video.c 2006-01-11
>>>19:28:02.000000000 +0000
>>>+++ v4l-dvb.test/linux/drivers/media/video/cx88/cx88-video.c
>>>2006-01-21 14:55:19.000000000 +0000
>>> (at) (at) -267,7 +267,7 (at) (at)
>>> .minimum = 0,
>>> .maximum = 0xff,
>>> .step = 1,
>>>- .default_value = 0x3f,
>>>+ .default_value = 0x39,
>>
> [snip]
>
> We have decided to leave the default value at 0x3f, regardless of what
> the datasheet says, regardless of what the other drivers are doing.
> This is a tiny change, and really isnt necessary, considering that 0x3f
> is truly 50% of 0x7f , which SHOULD be the maximum value...
>
It doesn't matter, I won't mind any of these two values.
> However, the maximum value for contrast is currently 0xff, which is
> utterly rediculous if you ask me, unless you guys like to watch
> television shows starring polar bears running through snowstorms.
>
I would like to notice, that I still have code to use full range and
have characteristic close to Your solution in most of the range.
It uses linear function in 0%-50% range(with coefficient 1/2), and
square function for rest of range with coefficients adjusted to have
contiguous differential in meet point. In practice in range 0%-80%
everything is almost like in Your/Jelle solution and rest of range is
available for any extreme need.
This solution was rejected, because it needs some global shadow value to
remember passed value before interpolation, as reverse transformation is
not accurate (needed in get function).
Besides it is quite radical solution, and changes slightly the way
contrast behaves (for 0-75% it's almost unnoticeable anyway).
Personally, I don't care what it would be. I can't remember using
brightness or other controls in other cases then testing code for such
discussions (except configuring tv-app for the first run).
> I propose the following patch (also located at
> http://linuxtv.org/~mkrufky/pending/cx88-contrast-max-fix.patch) :
>
> --- linux/drivers/media/video/cx88/cx88-video.c 10 Nov 2005 12:40:42 -0000 1.102
> +++ linux/drivers/media/video/cx88/cx88-video.c 8 Dec 2005 12:55:45 -0000
> (at) (at) -264,7 +264,7 (at) (at) static struct cx88_ctrl cx8800_ctls[] =
> .id = V4L2_CID_CONTRAST,
> .name = "Contrast",
> .minimum = 0,
> - .maximum = 0xff,
> + .maximum = 0x7f,
> .step = 1,
> .default_value = 0,
> .type = V4L2_CTRL_TYPE_INTEGER,
>
>
> Thanks to: Jelle Foks <jelle (at) foks.8m.com>
> Signed-off-by: Michael Krufky <mkrufky (at) linuxtv.org>
>
> I owe the credit of this patch to Jelle Foks, who actually started this
> whole effort of fixing this stuff in December. Unfortunately, Mauro
> will not allow me to apply this, because it decreases the contrast range
> by half. I think his reasoning is silly, because there will never be a
> need to use a setting above 0x7f.
>
"cx88_fix_controls_update.diff" patch _doesn't_ change contrast range.
Most of this patch was ready to apply by Mauro in December. Probably it
was my fault, it was delayed, because I found and fixed bug in
volume-balance controls, witch Mauro couldn't check without sound working.
> Ian, maybe you can try that patch and we can build a case and convince
> Mauro to allow it?
>
> I think the rest of this patch was interpolation stuff, which shouldnt
> be required given the trivial fix above. Once again, maybe
> interpolation is the way to go, but I think that simple changing max to
> 0x7f is sufficient, and WAAAAY cleaner.
>
The patch, Ian send (cx88_fix_controls_update.diff) _doesn't_ have any
interpolation code.
It doesn't break current situation with contrast range.
In fact, as I understand Ian intentions, this patch was an effect of
Mauro's, Ian's and mine discussion outside v4l-list in the middle of
December (as effect of Jelle's suggestions).
Mauro asked as to agree to add this patch (besides balance part) with
our (Ian and mine) signs. Then I only discovered problems with balance
and added code to handle this (there is undocumented behavior, that
balance attenuation + volume attenuation shouldn't be less then -63db,
or sound get at high volume for that channel).
Mauro couldn't check this balance problems, until alsa was finished.
Then he made impressive work on alsa module and this patch was forgotten.
> Anyhow, if you split your patch up into separate patches, one to handle
> contrast, one to handle volume, and the last for balance, maybe then it
> will be easier to push them through the beaurocracy.
Personally, I don't see any problem in applying patch, because it
doesn't break contrast range and doesn't add any new features breaking
current state.
To sum up, this patch _does_:
- fix table overflow for shadow registers; currently, when set balance
to odd (or even, I don't remember) value, in logs appears
"irq pci [0x1] vid*"
messages, because shadow table is to small, and irq_mask is overwritten!!
- fix default values for hue and brightness; current ones when gives b&w
dark image; to reproduce, just start kdetv and push "defaults" button in
controls window
- fix balance problems; currently, when used balance and volume, for
some range there is unwanted boost in audio volume; to reproduce, use
kdetv and set volume to 30%, and play with balance. In some range volume
goes down for given channel, but at some value volume in this channel
gets maximum and for further values it goes down once more. Volume and
balance attenuation for given channel is summed out and values below
-63db are internally(on chip) masked with 0x3f value (that's my
interpretation).
- fix temporary debug code (temporary for almost two months ;) ), which
causes excessive amount of messages in log, no matter what module debug
parameter value,
- moved CORE_IOCTL messages from level 1 to level 2; now it is generated
too often for that level (also temporary ;) ), when compared to other
messages.
- revert brightness default value to one from datasheet (I don't care if
it get included in v4l-dvb)
I checked latest v4l-dvb from hg, and above problems are fixed with this
patch.
So this patch mostly fixes bugs in current state, without breaking
mentioned contrast range (no mater what it should be in Your's, mine or
Mauro's opinion).
I don't see any reason to split it, but if You would insist, I have
prepared such version. You can add it, without default value change for
brightness, if You don't really like it.
Regards,
Marcin Rudowski
PS.
Because I also cc this letter to Mauro, I attached mentioned patch.
Maybe Mauro would add it personally, and end this discussion.
To pass any beaurocracy and just in case, here is also copy of Ian's
starting letter, which has all signs:
> Hello List,
> This patch was proposed by Marcin Rudowski about a month ago, and I
> think it has got lost in the activity at the time. I have thus remade
> the patch to work on the latest CVS copy (as of "2006-01-21 04:42
> mcisely" in the changelog).
>
> I have been running with this patch for some time now, and appreciate
> the default settings being correct by default, so I think it would be a
> good improvement for cx88 users.
>
> I have to stress that the hard work in this was done by Marcin and Mauro.
>
> The patch signature is as follows:
> ---------
> * linux/drivers/media/video/cx88/cx88-video.c: (get_control),
> (set_control), (init_controls), (cx88_do_ioctl):
> - Do some cleanups at cx88-video
> - Fixed a bug at balance control, when set to 0x40
> - improved debug messages
> - fixed default for contrast
>
> Signed-off-by: Ian Pickworth <ian (at) pickworth.me.uk>
> Signed-off-by: Marcin Rudowski <mar_rud (at) poczta.onet.pl>
> Signed-off-by: Mauro Carvalho Chehab <mchehab (at) brturbo.com.br>
> ------------
>
diff -Naur v4l-dvb/linux/drivers/media/video/cx88/cx88.h v4l-dvb.test/linux/drivers/media/video/cx88/cx88.h
--- v4l-dvb/linux/drivers/media/video/cx88/cx88.h 2006-01-08 03:39:03.000000000 +0000
+++ v4l-dvb.test/linux/drivers/media/video/cx88/cx88.h 2006-01-21 14:55:19.000000000 +0000
(at) (at) -66,7 +66,7 (at) (at)
/* need "shadow" registers for some write-only ones ... */
#define SHADOW_AUD_VOL_CTL 1
#define SHADOW_AUD_BAL_CTL 2
-#define SHADOW_MAX 2
+#define SHADOW_MAX 3
/* FM Radio deemphasis type */
enum cx88_deemph_type {
diff -Naur v4l-dvb/linux/drivers/media/video/cx88/cx88-video.c v4l-dvb.test/linux/drivers/media/video/cx88/cx88-video.c
--- v4l-dvb/linux/drivers/media/video/cx88/cx88-video.c 2006-01-11 19:28:02.000000000 +0000
+++ v4l-dvb.test/linux/drivers/media/video/cx88/cx88-video.c 2006-01-21 14:55:19.000000000 +0000
(at) (at) -253,7 +253,7 (at) (at)
.minimum = 0x00,
.maximum = 0xff,
.step = 1,
- .default_value = 0,
+ .default_value = 0x80,
.type = V4L2_CTRL_TYPE_INTEGER,
},
.off = 128,
(at) (at) -267,7 +267,7 (at) (at)
.minimum = 0,
.maximum = 0xff,
.step = 1,
- .default_value = 0x3f,
+ .default_value = 0x39,
.type = V4L2_CTRL_TYPE_INTEGER,
},
.off = 0,
(at) (at) -281,7 +281,7 (at) (at)
.minimum = 0,
.maximum = 0xff,
.step = 1,
- .default_value = 0,
+ .default_value = 0x80,
.type = V4L2_CTRL_TYPE_INTEGER,
},
.off = 128,
(at) (at) -323,10 +323,10 (at) (at)
.v = {
.id = V4L2_CID_AUDIO_VOLUME,
.name = "Volume",
- .minimum = 0,
- .maximum = 0x3f,
+ .minimum = 0, /* -63 dB */
+ .maximum = 0x3f, /* 0 dB */
.step = 1,
- .default_value = 0x1f,
+ .default_value = 0x3f,
.type = V4L2_CTRL_TYPE_INTEGER,
},
.reg = AUD_VOL_CTL,
(at) (at) -1160,7 +1160,8 (at) (at)
value = c->sreg ? cx_sread(c->sreg) : cx_read(c->reg);
switch (ctl->id) {
case V4L2_CID_AUDIO_BALANCE:
- ctl->value = (value & 0x40) ? (value & 0x3f) : (0x40 - (value & 0x3f));
+ ctl->value = ((value & 0x7f) < 0x40) ? ((value & 0x7f) + 0x40)
+ : (0x7f - (value & 0x7f));
break;
case V4L2_CID_AUDIO_VOLUME:
ctl->value = 0x3f - (value & 0x3f);
(at) (at) -1169,9 +1170,9 (at) (at)
ctl->value = ((value + (c->off << c->shift)) & c->mask) >> c->shift;
break;
}
- printk("get_control id=0x%X reg=0x%02x val=0x%02x (mask 0x%02x)%s\n",
- ctl->id, c->reg, ctl->value,
- c->mask, c->sreg ? " [shadowed]" : "");
+ dprintk(1,"get_control id=0x%X(%s) ctrl=0x%02x, reg=0x%02x val=0x%02x (mask 0x%02x)%s\n",
+ ctl->id, c->v.name, ctl->value, c->reg,
+ value,c->mask, c->sreg ? " [shadowed]" : "");
return 0;
}
(at) (at) -1181,6 +1182,7 (at) (at)
/* struct cx88_core *core = dev->core; */
struct cx88_ctrl *c = NULL;
u32 value,mask;
+ u32 tmp_vol_value;
int i;
for (i = 0; i < CX8800_CTLS; i++) {
if (cx8800_ctls[i].v.id == ctl->id) {
(at) (at) -1205,15 +1207,30 (at) (at)
mask=c->mask;
switch (ctl->id) {
case V4L2_CID_AUDIO_BALANCE:
- value = (ctl->value < 0x40) ? (0x40 - ctl->value) : ctl->value;
+ /* bits [0:5] represents attenaution (0:-63dB)
+ attenaution channel selection: ([6]==1) ? right : left */
+ value = (ctl->value < 0x40) ? (0x7f - ctl->value) : (ctl->value - 0x40);
+ /* (balance + volume) shouldn't be more then -63dB */
+ tmp_vol_value = cx_sread(SHADOW_AUD_VOL_CTL) & 0x3f;
+ if ( (value & 0x3f) + tmp_vol_value > 0x3f )
+ value = (value & 0x40) | (0x3f - tmp_vol_value);
break;
case V4L2_CID_AUDIO_VOLUME:
value = 0x3f - (ctl->value & 0x3f);
+
+ /* (balance + volume) shouldn't be more then -63dB */
+ tmp_vol_value = cx_sread(SHADOW_AUD_BAL_CTL) & 0x7f;
+ if ( (tmp_vol_value & 0x3f) + value > 0x3f ) {
+ tmp_vol_value = (tmp_vol_value & 0x40) | (0x3f - value);
+ dprintk(1,"set_control Balance correction: reg=0x%02x val=0x%02x (mask 0x7f)[shadowed]\n",
+ AUD_BAL_CTL, tmp_vol_value);
+ cx_sandor(SHADOW_AUD_BAL_CTL, AUD_BAL_CTL, 0x7f, tmp_vol_value);
+ }
break;
case V4L2_CID_SATURATION:
/* special v_sat handling */
- value = ((ctl->value - c->off) << c->shift) & c->mask;
+ value = ctl->value;
if (core->tvnorm->id & V4L2_STD_SECAM) {
/* For SECAM, both U and V sat should be equal */
(at) (at) -1228,9 +1245,9 (at) (at)
value = ((ctl->value - c->off) << c->shift) & c->mask;
break;
}
- printk("set_control id=0x%X reg=0x%02x val=0x%02x (mask 0x%02x)%s\n",
- ctl->id, c->reg, value,
- mask, c->sreg ? " [shadowed]" : "");
+ dprintk(1,"set_control id=0x%X(%s) ctrl=0x%02x, reg=0x%02x val=0x%02x (mask 0x%02x)%s\n",
+ ctl->id, c->v.name, ctl->value, c->reg, value,
+ mask, c->sreg ? " [shadowed]" : "");
if (c->sreg) {
cx_sandor(c->sreg, c->reg, mask, value);
} else {
(at) (at) -1246,8 +1263,7 (at) (at)
for (i = 0; i < CX8800_CTLS; i++) {
ctrl.id=cx8800_ctls[i].v.id;
- ctrl.value=cx8800_ctls[i].v.default_value
- +cx8800_ctls[i].off;
+ ctrl.value=cx8800_ctls[i].v.default_value;
set_control(core, &ctrl);
}
}
(at) (at) -1513,7 +1529,7 (at) (at)
{
int err;
- dprintk( 1, "CORE IOCTL: 0x%x\n", cmd );
+ dprintk(2, "CORE IOCTL: 0x%x\n", cmd );
if (video_debug > 1)
v4l_print_ioctl(core->name,cmd);
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request (at) redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list