Re: [patch][saa7134] do not change mute state for capturing audio

2011-12-03 Thread Stas Sergeev

Hello.

24.09.2011 16:48, Mauro Carvalho Chehab wrote:

A first scan at driver's init can be removed, IMO.

Done, sorry for the delay.
Patch is attached.

The attached patch fixes the automute logic of saa7134.
It avoids the white noise on the pulseaudio startup.
(pulseaudio reads the saa7134 alsa device on startup)

Signed-off-by: Stas Sergeev s...@users.sourceforge.net
From d8c8a05449b06ee7599e7c7d9e8aaeaa07d0fadb Mon Sep 17 00:00:00 2001
From: Stas Sergeev s...@users.sourceforge.net
Date: Sun, 4 Dec 2011 00:32:06 +0400
Subject: [PATCH] [saa7134] fix automute logic

---
 drivers/media/video/saa7134/saa7134-core.c|1 -
 drivers/media/video/saa7134/saa7134-tvaudio.c |   63 ++--
 drivers/media/video/saa7134/saa7134-video.c   |2 +
 drivers/media/video/saa7134/saa7134.h |1 +
 4 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/drivers/media/video/saa7134/saa7134-core.c 
b/drivers/media/video/saa7134/saa7134-core.c
index ca65cda..5fbb4e4 100644
--- a/drivers/media/video/saa7134/saa7134-core.c
+++ b/drivers/media/video/saa7134/saa7134-core.c
@@ -1263,7 +1263,6 @@ static int saa7134_resume(struct pci_dev *pci_dev)
saa7134_tvaudio_setmute(dev);
saa7134_tvaudio_setvolume(dev, dev-ctl_volume);
saa7134_tvaudio_init(dev);
-   saa7134_tvaudio_do_scan(dev);
saa7134_enable_i2s(dev);
saa7134_hw_enable2(dev);
 
diff --git a/drivers/media/video/saa7134/saa7134-tvaudio.c 
b/drivers/media/video/saa7134/saa7134-tvaudio.c
index 57e646b..ec1df6f 100644
--- a/drivers/media/video/saa7134/saa7134-tvaudio.c
+++ b/drivers/media/video/saa7134/saa7134-tvaudio.c
@@ -332,6 +332,13 @@ static int tvaudio_checkcarrier(struct saa7134_dev *dev, 
struct mainscan *scan)
 {
__s32 left,right,value;
 
+   if (!(dev-tvnorm-id  scan-std)) {
+   value = 0;
+   dprintk(skipping %d.%03d MHz [%4s]\n,
+   scan-carr / 1000, scan-carr % 1000, scan-name);
+   return 0;
+   }
+
if (audio_debug  1) {
int i;
dprintk(debug %d:,scan-carr);
@@ -348,30 +355,25 @@ static int tvaudio_checkcarrier(struct saa7134_dev *dev, 
struct mainscan *scan)
}
printk(\n);
}
-   if (dev-tvnorm-id  scan-std) {
-   tvaudio_setcarrier(dev,scan-carr-90,scan-carr-90);
-   saa_readl(SAA7134_LEVEL_READOUT1  2);
-   if (tvaudio_sleep(dev,SCAN_SAMPLE_DELAY))
-   return -1;
-   left = saa_readl(SAA7134_LEVEL_READOUT1  2);
-
-   tvaudio_setcarrier(dev,scan-carr+90,scan-carr+90);
-   saa_readl(SAA7134_LEVEL_READOUT1  2);
-   if (tvaudio_sleep(dev,SCAN_SAMPLE_DELAY))
-   return -1;
-   right = saa_readl(SAA7134_LEVEL_READOUT1  2);
-
-   left = 16;
-   right = 16;
-   value = left  right ? left - right : right - left;
-   dprintk(scanning %d.%03d MHz [%4s] =  dc is %5d [%d/%d]\n,
-   scan-carr / 1000, scan-carr % 1000,
-   scan-name, value, left, right);
-   } else {
-   value = 0;
-   dprintk(skipping %d.%03d MHz [%4s]\n,
-   scan-carr / 1000, scan-carr % 1000, scan-name);
-   }
+
+   tvaudio_setcarrier(dev,scan-carr-90,scan-carr-90);
+   saa_readl(SAA7134_LEVEL_READOUT1  2);
+   if (tvaudio_sleep(dev,SCAN_SAMPLE_DELAY))
+   return -1;
+   left = saa_readl(SAA7134_LEVEL_READOUT1  2);
+
+   tvaudio_setcarrier(dev,scan-carr+90,scan-carr+90);
+   saa_readl(SAA7134_LEVEL_READOUT1  2);
+   if (tvaudio_sleep(dev,SCAN_SAMPLE_DELAY))
+   return -1;
+   right = saa_readl(SAA7134_LEVEL_READOUT1  2);
+
+   left = 16;
+   right = 16;
+   value = left  right ? left - right : right - left;
+   dprintk(scanning %d.%03d MHz [%4s] =  dc is %5d [%d/%d]\n,
+   scan-carr / 1000, scan-carr % 1000,
+   scan-name, value, left, right);
return value;
 }
 
@@ -546,6 +548,7 @@ static int tvaudio_thread(void *data)
dev-tvnorm-name, carrier/1000, carrier%1000,
max1, max2);
dev-last_carrier = carrier;
+   dev-automute = 0;
 
} else if (0 != dev-last_carrier) {
/* no carrier -- try last detected one as fallback */
@@ -553,6 +556,7 @@ static int tvaudio_thread(void *data)
dprintk(audio carrier scan failed, 
using %d.%03d MHz [last detected]\n,
carrier/1000, carrier%1000);
+   dev-automute = 1;
 
} else {
/* no carrier + no fallback -- use default */
@@ -560,9 +564,9 @@ static int tvaudio_thread(void *data

Re: [patch][saa7134] do not change mute state for capturing audio

2011-09-24 Thread Stas Sergeev

24.09.2011 14:57, Mauro Carvalho Chehab wrote:

Please, one patch per email. Patchwork (or any kernel maintainer script)
won't catch more than one patch per email. See:

Sorry about that.


With respect to this patch:
http://patchwork.linuxtv.org/patch/7941/

I don't see any sense on it. Video standard selection is done by software,
when a standards mask is passed via VIDIOC_S_STD ioctl. Drivers should not
mess it with modprobe hacks.

Yes, but we already have secam= option, and
also the first scan, that is being done on driver
init, scans too much without that option, and
sometimes, unfortunately, detects the PAL carrier
for me.
By limiting it to secam, I avoid the problem and
shorten the scan time.
But this patch is not very important, so do whatever
you think necessary with it.


I'll comment later http://patchwork.linuxtv.org/patch/7940/. It seems to be
going into the right direction, but I need to take a deeper code inspection
and maybe do some tests here.

Thanks!
Of course, in my view, the _only_ right direction is
to export the mute control to the alsa mixer and then
fix mplayer. But at least I'm glad I've managed to
find the hack that satisfies your opinion and works
around the problem at the same time.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-09-24 Thread Stas Sergeev

24.09.2011 16:05, Mauro Carvalho Chehab wrote:


Better to post it as a separate patch, and to simplify the code with:

diff --git a/drivers/media/video/saa7134/saa7134-tvaudio.c 
b/drivers/media/video/saa7134/saa7134-tvaudio.c
index 57e646b..a61ed1e 100644
--- a/drivers/media/video/saa7134/saa7134-tvaudio.c
+++ b/drivers/media/video/saa7134/saa7134-tvaudio.c
@@ -332,6 +332,12 @@ static int tvaudio_checkcarrier(struct saa7134_dev *dev, 
struct mainscan *scan)
  {
__s32 left,right,value;

+   if (!dev-tvnorm-id  scan-std)) {

Missing open bracket?


@@ -546,6 +546,7 @@ static int tvaudio_thread(void *data)
dev-tvnorm-name, carrier/1000, carrier%1000,
max1, max2);
dev-last_carrier = carrier;
+   dev-automute = !(dev-thread.scan1  1);

Why?

Unfortunately, that's the trick. :(



If the carrier is good, this should be enough:

dev-automute = 0;

Unfortunately, sometimes it misdetects.
Testing dev-thread.scan1 means that at least
the first scan, done on the driver init, won't
unmute.
So either that, or this whole patch is unhelpful.
I realize that this is a dirty hack, yes.

The rest looked sane on my eyes, but I didn't double-checked it by 
running on my cards. Had you test calling it with just a single 
standard, and with a multiple standards mask? 

With just a single standard. That's the problem too.
There are the fallbacks, like last_carrier etc, and do we
need to unmute there or not? :(

 The right fix that pulseaudio should not touch at the audio mixers 
for the video boards.

That's where we disagree.
I wonder what other people think.
I don't see the compelling reason for making the
alsa interface to the v4l devs a special case. If there
is just a mute control exported, there is no more a special
case, and no more hacks and problems.

 Not all boards have an audio carrier detection like saa7134.
Having the mute control exported would make this
not a problem.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-09-24 Thread Stas Sergeev

24.09.2011 16:12, Mauro Carvalho Chehab wrote:

The scan audio logic only enables multiple audio standard detection if the 
userspace
application tells it to do.

No: the _first_ scan is done on the driver init.
It is a multi-standard one, and a long one, too.
Do we need it at all? It seems to me the results
of that scan are not even used, or what am I
missing?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-09-24 Thread Stas Sergeev

24.09.2011 16:48, Mauro Carvalho Chehab wrote:

A first scan at driver's init can be removed, IMO.

OK, that's the great news.
Will write a new patch then.

 There's nothing the driver can do if the hardware
 missdetects a carrier. Dirty tricks to try solving it
 are not good, as they'll do the wrong thing on some situations.
Well, if we assume the first scan can be removed,
then we also assume the previous dirty trick is
harmless, as it affects only the first scan. But I'll
better remove both the trick and the first scan then,
as the fewer the hacks, the better the code.

 If someone is using the board on an environment
 without udev and pulseaudio, this trick will break the first tuning.
I feel this somehow contradicts with your suggestion
to remove the first scan, so could you clarify?

 Well, if you think that this would solve, then just write a patch
 exporting the mute control via ALSA. I have no problems with that.
That would solve all the problems, but only if:
1. The mplayer is then moved to the use of that new
control to not depend on the autounmute hack.
I can write the patch for that too.
2. Make sure all the other apps are fixed the same way
(I hope there are none though)
3. The autounmute hack is then removed. (no
regressions if steps 1 and 2 are carefully done)

If you are fine with that plan, then I'll try to find
the time and do the things that way. Otherwise,
I'll remove the first scan, and that will do the trick
in a simpler, though less cleaner way.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-09-24 Thread Stas Sergeev

24.09.2011 19:09, Mauro Carvalho Chehab wrote:

If someone is using the board on an environment
without udev and pulseaudio, this trick will break the first tuning.

I feel this somehow contradicts with your suggestion
to remove the first scan, so could you clarify?

What I meant to say is that both udev and pulseaudio opens the device,
and these might initialize the audio thread. The driver should be able
to work the same way with or without the first open by udev/pulseaudio.

But the first scan I was referring to, and am going
to remove, happens not on the device open, but on
the driver init (modprobe time).
open()s are safe, fortunately.

The autounmute is not a hack. It is a logic to suppress audio when the
audio carrier is not detected. It should not be removed.

You are confusing the automute and autoUNmute.
Autounmute is a must-die hack, and we only need
to fix mplayer first. Automute just needs a fix.
Though I'd personally remove the automute too, by
exporting some interface for an app to query the
signal strength... but that's another story. :)


I'm not sure if it is safe to make mplayer to use the audio mixer.

Why, if otherwise it already uses alsa in our case?
The mixer control is just another part of an alsa
interface, and it is already exported to the v4l apps, so...


  It
is probably a good idea doing that, as it will also work fine with webcams
that provide alsa inputs.

And will make pulseaudio happy, that's for sure. :)
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-09-18 Thread Stas Sergeev

Hi Mauro, I've finally found the time (and an energy)
to go look into the automute breakage.
With the attached automute fix I no longer have
any problems with pulseaudio.
I also attached the patch that introduces an std
option to limit the scan list, resulting in a faster scan.
It is completely unrelated to the automute one, it is
here just in case.
What do you think?

From ccdfa126e98b5484f4a08de591ac8d89f775251c Mon Sep 17 00:00:00 2001
From: Stas Sergeev s...@users.sourceforge.net
Date: Sun, 18 Sep 2011 19:06:21 +0400
Subject: [PATCH 1/2] saa7134: fix automute

---
 drivers/media/video/saa7134/saa7134-tvaudio.c |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/saa7134/saa7134-tvaudio.c b/drivers/media/video/saa7134/saa7134-tvaudio.c
index 57e646b..62a6287 100644
--- a/drivers/media/video/saa7134/saa7134-tvaudio.c
+++ b/drivers/media/video/saa7134/saa7134-tvaudio.c
@@ -332,7 +332,7 @@ static int tvaudio_checkcarrier(struct saa7134_dev *dev, struct mainscan *scan)
 {
 	__s32 left,right,value;
 
-	if (audio_debug  1) {
+	if (audio_debug  1  (dev-tvnorm-id  scan-std)) {
 		int i;
 		dprintk(debug %d:,scan-carr);
 		for (i = -150; i = 150; i += 30) {
@@ -546,6 +546,7 @@ static int tvaudio_thread(void *data)
 dev-tvnorm-name, carrier/1000, carrier%1000,
 max1, max2);
 			dev-last_carrier = carrier;
+			dev-automute = !(dev-thread.scan1  1);
 
 		} else if (0 != dev-last_carrier) {
 			/* no carrier -- try last detected one as fallback */
@@ -553,6 +554,7 @@ static int tvaudio_thread(void *data)
 			dprintk(audio carrier scan failed, 
 using %d.%03d MHz [last detected]\n,
 carrier/1000, carrier%1000);
+			dev-automute = 1;
 
 		} else {
 			/* no carrier + no fallback -- use default */
@@ -560,9 +562,9 @@ static int tvaudio_thread(void *data)
 			dprintk(audio carrier scan failed, 
 using %d.%03d MHz [default]\n,
 carrier/1000, carrier%1000);
+			dev-automute = 1;
 		}
 		tvaudio_setcarrier(dev,carrier,carrier);
-		dev-automute = 0;
 		saa_andorb(SAA7134_STEREO_DAC_OUTPUT_SELECT, 0x30, 0x00);
 		saa7134_tvaudio_setmute(dev);
 		/* find the exact tv audio norm */
@@ -1020,6 +1022,7 @@ int saa7134_tvaudio_init2(struct saa7134_dev *dev)
 	}
 
 	dev-thread.thread = NULL;
+	dev-thread.scan1 = dev-thread.scan2 = 0;
 	if (my_thread) {
 		saa7134_tvaudio_init(dev);
 		/* start tvaudio thread */
-- 
1.7.6

From 70709f12f7161c98cb7ebae104b520dc30c6bd53 Mon Sep 17 00:00:00 2001
From: Stas Sergeev s...@users.sourceforge.net
Date: Sun, 18 Sep 2011 19:13:05 +0400
Subject: [PATCH 2/2] saa7134: introduce std module parameter to force video
 std

---
 drivers/media/video/saa7134/saa7134-video.c |   39 --
 drivers/media/video/saa7134/saa7134.h   |1 +
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/saa7134/saa7134-video.c b/drivers/media/video/saa7134/saa7134-video.c
index 776ba2d..04ac7de 100644
--- a/drivers/media/video/saa7134/saa7134-video.c
+++ b/drivers/media/video/saa7134/saa7134-video.c
@@ -40,12 +40,15 @@ static unsigned int noninterlaced; /* 0 */
 static unsigned int gbufsize  = 720*576*4;
 static unsigned int gbufsize_max  = 720*576*4;
 static char secam[] = --;
+static char std[16] = --;
 module_param(video_debug, int, 0644);
 MODULE_PARM_DESC(video_debug,enable debug messages [video]);
 module_param(gbuffers, int, 0444);
 MODULE_PARM_DESC(gbuffers,number of capture buffers, range 2-32);
 module_param(noninterlaced, int, 0644);
 MODULE_PARM_DESC(noninterlaced,capture non interlaced video);
+module_param_string(std, std, sizeof(std), 0644);
+MODULE_PARM_DESC(secam, force TV standard, either PAL, SECAM or NTSC);
 module_param_string(secam, secam, sizeof(secam), 0644);
 MODULE_PARM_DESC(secam, force SECAM variant, either DK,L or Lc);
 
@@ -1847,14 +1850,20 @@ int saa7134_s_std_internal(struct saa7134_dev *dev, struct saa7134_fh *fh, v4l2_
 		return -EBUSY;
 	}
 
-	for (i = 0; i  TVNORMS; i++)
+	for (i = 0; i  TVNORMS; i++) {
+		if (!tvnorms[i].enabled)
+			continue;
 		if (*id == tvnorms[i].id)
 			break;
+	}
 
 	if (i == TVNORMS)
-		for (i = 0; i  TVNORMS; i++)
+		for (i = 0; i  TVNORMS; i++) {
+			if (!tvnorms[i].enabled)
+continue;
 			if (*id  tvnorms[i].id)
 break;
+		}
 	if (i == TVNORMS)
 		return -EINVAL;
 
@@ -1871,6 +1880,8 @@ int saa7134_s_std_internal(struct saa7134_dev *dev, struct saa7134_fh *fh, v4l2_
 fixup = V4L2_STD_SECAM;
 		}
 		for (i = 0; i  TVNORMS; i++) {
+			if (!tvnorms[i].enabled)
+continue;
 			if (fixup == tvnorms[i].id)
 break;
 		}
@@ -2579,8 +2590,30 @@ int saa7134_videoport_init(struct saa7134_dev *dev)
 
 int saa7134_video_init2(struct saa7134_dev *dev)
 {
+	int i, idx = -1;
+	for (i = 0; i  TVNORMS; i++) {
+		int enable = (std[0] == '-' || strncasecmp(std,
+tvnorms[i].name, strlen(std)) == 0);
+
+		if (secam[0] != '-') {
+			char buf[16] = SECAM;
+			if (strncasecmp(buf, tvnorms[i].name,
+	strlen(buf)) == 0

Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-25 Thread Stas Sergeev

24.07.2011 22:36, Mauro Carvalho Chehab wrote:

The automute code works fine. Maybe you have a strong interference
at the default tuning frequency, leading into saa7134 miss-detection.

OK, so my accusation to the automute code is
that it gets disabled unconditionally, no matter
have the scan failed or succeeded. Also, since
that scan is done on driver init, the automute
state stands no chance to survive: it is getting
disabled unconditionally, on the driver init.
Do we agree that this is a bug?
Do we agree that fixing it will also fix the PA problem,
or, at the very least, will advance us a lot in getting
it fixed?
If so, can you take a look into fixing that code?
It seems the automute code is rather fragile right
now, I'd better not touch it if you have some time
to take a look.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-24 Thread Stas Sergeev

23.07.2011 19:09, Mauro Carvalho Chehab wrote:

  In this case, it will not be autounmuted after tuning.
Hard to tell about your solution without seeing a patch.

OK, it turns out the automute code is already there,
but it doesn't work. The driver for some reasons
starts the scan on initialization, finds the carrier:
---
saa7134[0]/audio: found PAL main sound carrier @ 6.000 MHz [3969/324]
---
and, because of that, disables the automute. If the
real mute is not enabled at that point, you get the
white noise right away.

Since I have no idea why it finds some carrier, I can't
fix that in any way. Or, maybe, not to call the scan
on driver init? What will that break?

Anyway, as long as the automute code is broken,
we should either start fixing it, or fix PA, or fix mplayer...
Dunno. I wonder how come so many bugs left unfixed
for so long, resulting in a white noise to people...
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-24 Thread Stas Sergeev

24.07.2011 22:36, Mauro Carvalho Chehab wrote:

So, only people that has saa7134 with alsa stream that opted
to wire the saa7134 device to the sound card, and with a strong
interference at the default tunning frequency (400 MHz) would
notice a problem.

No, the strong interference thing have nothing to
do with it, I think. My card detects signal sometimes,
not always. Otherwise it says this:
---
saa7134[0]/audio: audio carrier scan failed, using 5.500 MHz [default]
---
yet the moise is still there.
If you look into a tvaudio_thread() function, you'll
notice that it disables automute _unconditionally_!
saa7134_tvaudio_do_scan() also disables automute
unconditionally.
That's why I think there are bugs. Can we start
from fixing at least this, and see what happens then?


Since I have no idea why it finds some carrier, I can't
fix that in any way. Or, maybe, not to call the scan
on driver init? What will that break?

Analog tuners need to be tuned at the device init on a high frequency
according with their datasheets, otherwise the PLL may fail.

OK.
Maybe, not disabling the automute when the scan
was started at init, rather than when it was requested
by an app?


You're the first one that reported it, and the code is there for _years_.
So, this is not a commonly noticed problem at all.

I am only the first one who reported it _to that list_.
I think most other reports were against pulseaudio.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-23 Thread Stas Sergeev

23.07.2011 05:28, Mauro Carvalho Chehab wrote:

Mplayer was just one example of an application that I know
it doesn't call V4L2_CID_AUDIO_MUTE to unmute.

I would suggest fixing all such an apps, even if we
are not going to change that in the driver.


Your approach of moving it to VIDIOC_S_FREQUENCY (if I
understood well) won't work, as, every time someone would
change the channel, it will be unmuted, causing troubles
on applications like scantv (part of xawtv).

But how can scantv (or anything else) rely on the
fact that the board was muted when that app starts?
I guess it can't, and mutes it explicitly first, no?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-23 Thread Stas Sergeev

23.07.2011 17:06, Mauro Carvalho Chehab wrote:

I would suggest fixing all such an apps, even if we
are not going to change that in the driver.

If application needs to change due to a patch, this is a
regression,

I said even if we are not going to change that in the
driver, which, imho, removes any ambiguity from my
phrase.


But how can scantv (or anything else) rely on the
fact that the board was muted when that app starts?
I guess it can't, and mutes it explicitly first, no?

Even if it mutes, every time a channel is changed, it
will be unmuted, if you put such unmute logic at
VIDIOC_S_FREQUENCY.

As I said, I propose the automute state to be a separate,
_third_ state. mute/unmute/automute.
Automute state is only set initially, but if the app
explicitly sets any other state, it is no longer affected.
Since an app can't rely on the state before it was
started, it should set the mute state explicitly first.
In this case, it will not be autounmuted after tuning.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-23 Thread Stas Sergeev

23.07.2011 19:09, Mauro Carvalho Chehab wrote:

As I said, I propose the automute state to be a separate,
_third_ state. mute/unmute/automute.
Automute state is only set initially, but if the app
explicitly sets any other state, it is no longer affected.
Since an app can't rely on the state before it was
started, it should set the mute state explicitly first.
In this case, it will not be autounmuted after tuning.

Hard to tell about your solution without seeing a patch.

I can try making this patch now only if we agree
on the technique first.


Not sure if this will be consistent, especially if PA
restarts for whatever reason (X restart? manual restart?).

I mean, this automute is set initially for every
new opening of the device node. So on every start
it will still have an automute mode.


Anyway, we're discussing a lot for a kernel fix for PA,
while the right thing to do is to fix PA itself.

I think both parts will better be fixed ideally, but
right now PA will probably not be fixed soon.
If we can agree on the logic, then I may take a
look into coding the patch itself.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-23 Thread Stas Sergeev

23.07.2011 19:09, Mauro Carvalho Chehab wrote:

Anyway, we're discussing a lot for a kernel fix for PA,

Please note that right now we are discussing the
fix for mplayer or anything else that forgets to
just explicitly enable/disable the audio!
IMHO, that should better be fixed first.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-22 Thread Stas Sergeev
Ok, Mauro, so may I take your silence as an evidence
that this reiterating myth about the mplayer breakage
is just a myth?
Look, I spent time on investigating the problem, on
trying the different approaches to fix it, on explaining
the problem to you, etc. So maybe I deserve something
more than just a blunt NACK, lets fix real bugs reply
you initially did? :)
Note: that's the first time I got the nack without any
explanation in the very first reply, and with the false
explanations later. My patch doesn't break mplayer: it
can't, since mplayer does not use that interface at all.
And my patch fixes a real problem, so even if it is for
some reasons incorrect, it certainly deserves a better
treatment than the false claims.
I guess you are doing this in order to just push your
own patch, and you'll do that anyway, so this letter of
disappointment is going to be my last posting to that
thread, unless you decide to explain your nack after all. :)
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-22 Thread Stas Sergeev
22.07.2011 16:28, Mauro Carvalho Chehab wrote:
 In this specific case, applications like mplayer,
 using the alsa parameters for streaming will stop work, as mplayer
 won't touch at the mixer or at the V4L mute control. So,
 it will have the same practical effect of a kernel bug at the
 audio part of the driver.
Let me quote you again:
---
Some applications like mplayer don't use V4L2_CID_AUDIO_MUTE to unmute a
video device. They assume the current behavior that starting video also
unmutes audio.
---
Could you please explain how my patch breaks
starting video also unmutes audio? I haven't touched
anything related to starting video, so, if starting video
used to unmute audio, it will keep it that way.
Can you tell me how exactly I can reproduce that breakage?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-22 Thread Stas Sergeev
22.07.2011 16:49, Mauro Carvalho Chehab wrote:
 Let me rephase it:
 Some applications like mplayer don't use V4L2_CID_AUDIO_MUTE to unmute a
 video device. They assume the current behavior that starting audio on a
 video board also unmutes audio.
Could you please give me a command line I can use
to verify that? Or any pointers to the code, anything
to check?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-22 Thread Stas Sergeev

22.07.2011 17:03, Mauro Carvalho Chehab wrote:

Here, I add the following line at my .mplayer/config:

tv  = 
driver=v4l2:device=/dev/video0:norm=PAL-M:chanlist=us-bcast:alsa=1:adevice=hw.1:audiorate=32000:immediatemode=0:amode=1

Thanks for starting to answer what I was asking for over a week. :)
If this is the case (not verified yet), there may be the simple
automute logic that will fix that in an absense of an auto-unmute
in alsa.
Initially, the driver may be put in an auto-mute state.
It is mute until the tuner is tuned: after the tuner is tuned,
the audio gets immediately automatically unmuted.
If the app does not want this to happen, it may use
the V4L2_CID_AUDIO_MUTE before tuning, to put the device
in a permanent-mute state.
So, in short, I suggest to bind the auto-unmute to the
tuner tune, rather than to the capture start. And that
should be a separate, third mute state, automute. If the
app explicitly wants the mute or unmute, this automute
logic disables.
Do you know any app that will regress even with that?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-20 Thread Stas Sergeev
20.07.2011 14:32, Mauro Carvalho Chehab wrote:
 I won't keep discussing something that won't be merged, as it will
 cause regressions.
Please explain what regressions it will make!
I am asking that question over and over again, and
every time you either ignore it, or refer to an apps
that use v4l2 ioctls, which are unaffected.
I wonder why you don't want to explain what regressions
do you have in mind...

 If the application is starting streaming, audio should be expected on
 devices
 where the audio output is internally wired with the capture input.
 This seems to be the case of your device. There's nothing that can be
 done to fix a bad hardware design or the lack of enough information
 from the device manufacturer.
Well, until you explain the exact breakage of my proposal,
I won't trust this. :)

 I suspect, however, that not changing the GPIO's is a very bad idea, and
 it will actually break audio for devices with external GPIO-based input
 switches, but, as this version was already done, it might be useful for some
 tests. A version 3 will follow shortly.
I'll test at a week-end whatever we'll have to that date.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-20 Thread Stas Sergeev
20.07.2011 14:48, Mauro Carvalho Chehab wrote:
 Well, until you explain the exact breakage of my proposal,
 I won't trust this. :)
 I've said already: mplayer for example relies on such behavior to work. 
 Reverting
 it breaks mplayer. This is enough for me to NACK your patch.
What you said, was:
---
Some applications like mplayer don't use V4L2_CID_AUDIO_MUTE to unmute a
video
device. They assume the current behavior that starting video also
unmutes audio.
---
starting video also unmutes audio is what my patch
_does not touch_! And that certainly happens not even
in the alsa driver, but somewhere in the v4l2 code.

So, please please please, could you actually precisely
explain how exactly mplayer breaks with my patch?
That's the only thing I need! :))
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-19 Thread Stas Sergeev

19.07.2011 03:16, Lennart Poettering wrote:
ALSA doesn't really have a enumeration API which would allow us to get 
device properties without opening and configuring a device. In fact, 
we can't even figure out whether a device may be opened in duplex or 
simplex without opening it.


And that's why we have to probe audio devices, even if it sucks.

Hi Lennart, thanks for your opinion.

I am puzzled with the even if it sucks part, what
does it mean? I see 2 possible interpretations of it:

1. Even if it sucks with some drivers that have bugs,
like the saa7134_alsa one. If that interpretation is
what you implied, then could you please also evaluate
the fix like this one:
 http://www.spinics.net/lists/linux-media/msg35237.html

2. Even if it sucks in general. In this case, what solution
would you propose to get the problem of the white
noise fixed?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-19 Thread Stas Sergeev
19.07.2011 17:00, Mauro Carvalho Chehab wrote:
 Several video boards have the option of plugging a loop cable between
 the device output pin and the motherboard line in pin. So, if you start
 capturing, you'll also enabling the output of such pin, as the kernel
 driver has no way to know if the user decided to use a wire cable,
 instead
 of the ALSA PCM stream.
 So, if users with such cables are lucky, it will play something, but,
 on most cases, it will just tune into a non-existing station, and it will
 produce a white noise.
This needs to be clarified a bit (for Lennart).
Initially, before the board is tuned to some station,
the sound is wisely muted. It is muted for both the
capturing and the pass-through cable.
As far as I can tell, if you want to probe the card by
capturing, you can capture the silence, you don't need
any real sound to record.
The problem here is that the particular driver has a
nice code (or a hack) that unmutes both the capturing
and the pass-through cable when you capture anything.
From my POV, exactly that leads to the problem. Simply
removing that piece of code makes the peace in the world:
the app that tunes the board, also unmutes the sound anyway.

My question was and still is: do we need to search for
any other solution at all? Do we need to modify PA, if
it is entirely fine with capturing the silence for probing audio?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-19 Thread Stas Sergeev
19.07.2011 18:10, Mauro Carvalho Chehab wrote:
 As this is an USB device, in general, people don't connect the line out
 pin. So, typically, in order to unmute this particular device for TV, one
 should unmute both AC97 MONO and AC97 VIDEO, and mute AC97 LINE IN.

 If the application latter changes to SVideo, the AC97 VIDEO should be
 muted, and AC97 LINE IN should be unmuted.
Unless I am missing the point, you need some mixer control
that will just unmute the currently-configured things.
If you can unmute all the right things when an app just
starts capturing, then you can as well unmute the same
things by that _single_ mixer control.
And if the app changes the output to SVideo, as in your
example, you can first mute everything, and then unmute
the new lines, but only if the old lines were unmuted.
IMHO, that logic will not break the existing apps.

 Moving such logic to happen at userspace would be very complex, and will
 break existing applications.
If this is the case, then how does the simplest
xawtv's mute/unmute thing works with all these
boards right now? (not that I have checked it does,
but I hope so. :)
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-19 Thread Stas Sergeev
19.07.2011 19:27, Mauro Carvalho Chehab wrote:
 Unless I am missing the point, you need some mixer control
 that will just unmute the currently-configured things.
 If you can unmute all the right things when an app just
 starts capturing, then you can as well unmute the same
 things by that _single_ mixer control.
 And if the app changes the output to SVideo, as in your
 example, you can first mute everything, and then unmute
 the new lines, but only if the old lines were unmuted.
 IMHO, that logic will not break the existing apps.
 That is the current logic, except that we don't create an additional
 virtual mixer control like the one you've proposed via ALSA API.
Unless I am mistaken, this control is usually called a
Master Playback Switch in the alsa world.
So, am I right that the only problem is that it is not
exported to the user by some drivers right now?
And, if it is made exported, what will still prevent us
from dropping the auto-unmute stuff?

 Yet, as you may be aware of that, the V4L2 API offers a few audio
 controls
 (volume, mute, balance, bass, treble), that applies to the current 
 stream, on the drivers that provide them. So, a video application may opt to
 not control the alsa mixers directly, but, instead, use the V4L2 controls.
In this case, I think, the alsa mixer control should just
mirror the one of the v4l2 for the most cases. Maybe
for some boards they can actually do the different things -
doesn't matter right now though.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-19 Thread Stas Sergeev
19.07.2011 22:06, Mauro Carvalho Chehab wrote:
 Unless I am mistaken, this control is usually called a
 Master Playback Switch in the alsa world.
 No, you're mistaken: on most boards, you have only one volume control/switch,
 for capture. So, it would be a master capture switch,
Well, for such a cards we don't need to export
the additional element, they are fine already.
We can rename it to Master Capture Switch,
or may not.

  but I don't think
 that there's such alsa generic volume control. Even in the case where
 you have a volume control for the LINE OUT pin[1], in general, you also need 
 to
 unmute the capture, so, it would be a master capture and LINE OUT switch,
 and, for sure alsa currently not provide anything like that.
I think you can still call it a Master Capture Switch,
if it enables everything.

 So, am I right that the only problem is that it is not
 exported to the user by some drivers right now?
 No, you're mistaken again. Such master capture and LINE OUT switch type of 
 control
 _is_exported_ via the V4L2 API as V4L2_CID_AUDIO_MUTE. 
Sorry, I meant the _alsa_ drivers here.
So, to rephrase:

So, am I right that the only problem is that it is not
exported to the user by some _alsa_ drivers right now?


 Some applications like mplayer don't use V4L2_CID_AUDIO_MUTE to unmute a video
 device. They assume the current behavior that starting video also unmutes 
 audio.
 (mplayer is not symmetric with regard to the usage of this control, as it uses
 V4L2_CID_AUDIO_MUTE to mute the device after the end of a capture).

 So, changing the logic at the drivers will break existing applications.
I do not propose changing any V4L2 ioctls, my
change concerns only the alsa driver.

 It is probably doable to split the mute control for the LINE OUT pin from the
 mute control of the PCM capture. Such patch would make sense, as the alsa
 capture doesn't need to touch at the line out pin, but the patch should
 let V4L2_CID_AUDIO_MUTE control to affect both LINE OUT and PCM capture
 mutes, otherwise applications will break.
That's exactly what I was talking about from the very
beginning, saying that the single control currently controls
way too much, and providing an examples about 2 separate
controls. But... I haven't found the way to implement that,
not sure of this is possible at all. :(
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-19 Thread Stas Sergeev

19.07.2011 23:29, Mauro Carvalho Chehab wrote:

the additional element, they are fine already.
We can rename it to Master Capture Switch,
or may not.

Adding a new volume control that changes the mute values for the other controls
or renaming it don't solve anything.

The proposed solution is to have the mute
control, that can be valid for all the cards/drivers.
Presumably, it should have the similar name
for all of them, even though for some it will be
a virtual control that will control several items,
and for others - it should map directly to their
single mute control.
If we have such a mute control, any app can use
it, and the auto-unmute logic can be removed
from the alsa driver. v4l2 is left as it is now.
So that's the proposal, what problems can you see
with it?


So, am I right that the only problem is that it is not
exported to the user by some _alsa_ drivers right now?

I fail to see why this would be a problem.

But that was the problem _you_ named.
That is, that right now the app will have difficulties
unmuting the complex boards via the alsa interface,
because it will have to unmute several items instead
of one.
I propose to add the single item for that, except for
the drivers that already have only one mute switch.
With this, the problem you named, seems to be solved.
And then, perhaps, the auto-unmute logic can go away.
What am I missing?


It is doable, although it is probably not trivial.
Devices with saa7130 (PCI_DEVICE_ID_PHILIPS_SAA7130) doesn't enable the
alsa module, as they don't support I2S transfers, required for PCM audio.
So, we need to take care only on saa7133/4/5 devices.

The mute code is at saa7134-tvaudio.c, mute_input_7134() function. For
saa7134, it does:

 if (PCI_DEVICE_ID_PHILIPS_SAA7134 == dev-pci-device)
 /* 7134 mute */
 saa_writeb(SAA7134_AUDIO_MUTE_CTRL, mute ?
 SAA7134_MUTE_MASK |
 SAA7134_MUTE_ANALOG |
 SAA7134_MUTE_I2S :
 SAA7134_MUTE_MASK);

Clearly, there are two mute flags: SAA7134_MUTE_ANALOG and SAA7134_MUTE_I2S.

I was actually already playing with that piece of
code, and got no results. Will retry the next week-end
to see exactly why...
IIRC the problem was that this does not mute the
sound input from the back panel of the board, which
would then still go to the pass-through wire in case
you are capturing. The only way do mute it, was to
configure muxes the way you can't capture at the
same time. But I may be wrong with the recollections.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-19 Thread Stas Sergeev

20.07.2011 04:55, Mauro Carvalho Chehab wrote:

The proposed solution is to have the mute
control, that can be valid for all the cards/drivers.
Presumably, it should have the similar name
for all of them, even though for some it will be
a virtual control that will control several items,
and for others - it should map directly to their
single mute control.
If we have such a mute control, any app can use
it,

Any app can do it right now via the V4L2 api.

I am just following your logic, you said that
---
Moving such logic to happen at userspace would be very complex, and will
break existing applications.
---
To solve that, I proposed adding such mixer control
to where it is missing right now.
But if this is no longer a problem and the app
can just use v4l2 for that instead, then what still
keeps us from removing the auto-unmute things?


and the auto-unmute logic can be removed
from the alsa driver. v4l2 is left as it is now.

What is the sense of capturing data for a device that is not ready
for stream?

This may be a PA's hack, or a user's mistake, or
whatever, but whatever it is, it shouldn't lead to
any sounds from speakers. Just starting the capture,
willingly or by mistake, should never lead to any
sound from speakers, IMO. So that's the bug too.
And the simpler one to fix.


So that's the proposal, what problems can you see
with it?

Userspace application breakage is not allowed. A change like
that will break the existing applications like mplayer.

No, because, as you said, it uses v4l2, not alsa,
to unmute.
And my proposal only affects alsa, so what's the
breakage?


Maybe your device is not a saa7134. For saa7133/saa7135, the
mute/unmute seems to be done via GPIO, and via amux.

Yes, and that's exacly why unmuting only I2S
does nothing: the muxes are still set up for mute.


IIRC the problem was that this does not mute the
sound input from the back panel of the board, which
would then still go to the pass-through wire in case
you are capturing. The only way do mute it, was to
configure muxes the way you can't capture at the
same time. But I may be wrong with the recollections.

Well, the change seems to be simple, as we don't actually need to
split the mute. We just need to control the I2S input/output at
the alsa driver.

The enclosed patch probably does the trick (completely untested).

I'll be able to test it on avertv 307 the next week-end.
But what is the expected effect of that patch?
It looks much like mine: mostly just removes auto-unmute,
doing that in a not-so-obvious way.
The card is muted by setting up the muxes.
Now you unmute it by enabling I2S, but the muxes
are still set to mute, so nothing happens, and you
will capture the silence.
I think this patch is _correct_, as it removes the
auto-unmute logic; exactly what I proposed. :)
Just a slightly different implementation, unless
I am missing something obvious...
By the way, do you need to do

saa7134_i2s_mute(dev, mute);

from mute_input_7134() ? Maybe leaving that I2S
control entirely for alsa, and not touching it elsewhere?
The function itself can probably then be moved to
saa7134-alsa.c.

Thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-17 Thread Stas Sergeev

15.07.2011 05:38, Mauro Carvalho Chehab wrote:

If you want, feel free to propose a patch fixing that logic at saa7134, instead
of just removing it.

Hi, I've just verified that pulseaudio indeed does
the sound capturing on startup:
---
saa7134[0]/alsa: saa7134[0] at 0xfe8fb800 irq 22 registered as card 2
saa7134[0]/alsa: rec_start: afmt=2 ch=1  =  fmt=0xcd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=1  =  fmt=0xcd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=2  =  fmt=0xdd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=2  =  fmt=0xdd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=2  =  fmt=0xdd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=2  =  fmt=0xdd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=1  =  fmt=0xcd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=1  =  fmt=0xcd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=2  =  fmt=0xdd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=2  =  fmt=0xdd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=2  =  fmt=0xdd swap=-
saa7134[0]/alsa: rec_start: afmt=2 ch=2  =  fmt=0xdd swap=-
saa7134[0]/alsa: irq: field oops [even]
---

So your proposal is not going to fix anything at all.

Can we get back to discussing/applying mine then?
And if the other drivers has that autounmute logic,
then I suggest removing it there as well. You have
not named any use-case for it, so I think there is none.
I also think that the whole auto-unmute logic in your
drivers is entirely flawed: for instance, I don't think
recording from the sound card will automatically
unmute its line-in or something else, so you are probably
not following the generic alsa style here.
I am adding alsa-devel to CC to find out what they
think about that whole auto-unmute question.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-17 Thread Stas Sergeev

17.07.2011 15:51, Mauro Carvalho Chehab wrote:

(Added Lennart to the c/c list)
If pulseaudio is starting sound capture at startup, then it is either
a pulseaudio miss-configuration or a bug there.

Why?


I think that this is not the default for pulseaudio, though, as
you're the only one complaining about that, and I never saw such
behavior in the time I was using pulseaudio here.

I've seen such a problem mentioned on the russion
linux resource a few years ago... The reason why it
was never mentioned on that list, is probably that
noone tracked it down to the saa7134_alsa driver yet.
But maybe the reason is different, ok, lets see what
Lennart thinks.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-15 Thread Stas Sergeev

15.07.2011 05:38, Mauro Carvalho Chehab wrote:

In any case, all V4L drivers should have the same behavior on that matter.

I am not sure how exactly the other drivers behave,
and I agree they should behave more or less similar
(as long as the particular hw allows, not the case with
saa7134). But if we can't even agree on what the mixer
control should do, or whether the sound capture should
result in any sound from the speakers, then I would
suggest adding the alsa list to CC. After all, these
rules are set by them, not by you or me.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-14 Thread Stas Sergeev

15.07.2011 05:38, Mauro Carvalho Chehab wrote:

Huh? The first time, you said it were due to pulseaudio. Then, you

Yes: pulseaudio does some capturing at startup.
(or, possibly, just opens the capture device). Without
it, nothing bad happens, but I never said the bug is
in pulseaudio.


said it were due to xawtv,

No, I was mentioned xawtv as an app that have to
set everything up properly, before you can capture
anything else than the white noise.
So my point was, and is, that, before something like
xawtv sets up the tuner, unmuting the audio makes
_zero_ sense.


  and now you're blaming Xorg startup.

I am not _blaming_ it, just mentioning it.
Indeed, the pulseaudio starts on the xorg startup,
at least on fedora. So, from the mere user's point
of view, you start xorg and get the noise.


Starting X should not be touching on anything, as Xorg itself doesn't
have any code to handle an alsa device.

But, with some magic scripts, it starts pulseaudio.


The expected behavior of the driver should be to unmute the device only
if TV and/or radio starts streaming, and muting it at stream stop.

What does streaming means here, exactly?


Also, the alsa driver doesn't have any business to do when the
audio is wire connected, excepting by providing the mixer controls.

The problem is exactly here: the single mixer control
controls both the pass-through wire and the input
for capturing.
Be there the 2 separate controls, or be there a control
_only_ for the pass-through write, the problem would
not exist. But currently the single mixer control controls
too much.


The mute/unmute logic is there due to the fact that, nowadays, most boards
provide audio PCM output, and such setup is generally preferred, as it
doesn't require an extra cabling, and gives more quality to the audio, as
it avoids an extra Digital/Analog and Analog/Digital conversion, thus
reducing the quantization noise and any analog interferences.

Please clarify that part a bit.
How exactly the expected mute/unmute logic should
affect the pass-through wire, and how exactly should
it affect the PCM capture.


By looking at the alsa driver, the logic is muting/unmuting at device open
and not at device capture. So, it is not doing the expected behavior.

The proper fix seems to move that logic to capture start/stop. We need to

What if pulseaudio really captures something, rather than
just opens the device? Not that I have checked it does, but
it may be the case if he wants to calibrate some clocks by
recording something.
Why just recording from the alsa device, without feeding
the sound to the sound card, should ever produce any
sound from the speakers? No program in the world would
expect that behaveor, and the pulseaudio's case may or
may not be fixed by that (depends on luck, and, possibly,
the pulseaudio version), so why doing such a change?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-13 Thread Stas Sergeev

14.07.2011 00:53, Mauro Carvalho Chehab wrote:

When pulseaudio enables the audio capturing, the
driver unmutes the sound. But, if no app have properly
tuned the tuner yet, you get the white noise.
I think the capturing must not touch the mute state,
because, without tuning the tuner first, you can't capture
anything anyway.
Without this patch I am getting the white noise on every
xorg/pulseaudio startup, which made me to always think
that pulseaudio is a joke and will soon be removed. :)

Nack. We shouldn't patch a kernel driver due to an userspace bad behavior.

But I really think that the driver behaves badly here.
Suppose we had 2 separate mute switches: the input
mute, that mutes the signal as it just enters the saa
chip, and the output mute, that mutes only the output
of the tuner card, that is connected to the sound card's
line input.
With that configuration, we'd allow the alsa driver to
unmute only the input switch, so that it can record, but
leave the output switch still muted, so that the sound
not to come to the sound card directly.
Now that we don't have the output mute switch, we
allow the alsa driver to unmute not only the recording
that it may need, but also the sound output that goes
to the sound card! IMHO, this is the entirely unwanted
side effect, so I blame the saa driver, and not the pulseaudio.

There are also other things to consider:
1. You can't record anything (except for the white noise)
before some xawtv sets up everything. So what is the
use-case of the current (mis)behaveur?
2. The alsa driver, trying to manage the mute state on
its own, badly interwinds with the mute state of the
(xawtv) program. 2 programs cannot control the same
mute state for good, and of course the xawtv must have
the preference, as the alsa driver have no slightest
idea about the card's state.
3. The problem is very severe. Hearing the loud white
noise on every startup is not something the human can
easily tolerate. So deferring it for the unknown period
is simply not very productive.

Can you please name a few downsides of the approach
I proposed?
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch][saa7134] do not change mute state for capturing audio

2011-07-13 Thread Stas Sergeev

14.07.2011 02:00, Mauro Carvalho Chehab wrote:


Now that we don't have the output mute switch, we
allow the alsa driver to unmute not only the recording
that it may need, but also the sound output that goes
to the sound card! IMHO, this is the entirely unwanted
side effect, so I blame the saa driver, and not the pulseaudio.

Why this is unwanted? You shouldn't expect that the poor
users to control each mute control. They just need to control
one: the sound card outut.

Controlling the sound card output makes no sense
here: I don't want to mute the entire sound only when
I want to mute the TV-tuner.
On the other hand, why exactly would you unmute
the output when capturing? Obviously to allow the
capturing itself.
Why, at the same time, would you enable the pass-through
link to the sound card? Unwanted side-effect: it is
not needed for capturing, and it gives the noise.
That have to be fixed.
So: even if pulseaudio wants to record the white
noise for one reason or another, at least it doesn't
output it to the sound card, so what it does is perfectly
safe. Enabling the pass-through link to the sound card
is a bug here.


There are also other things to consider:
1. You can't record anything (except for the white noise)
before some xawtv sets up everything. So what is the
use-case of the current (mis)behaveur?

So is there a use-case?


If you're getting a white noise, then there's a bug either
at xawtv, at the driver or both. It is likely board-specific,
as, at least the last time I tested, saa7134 audio were working
properly.

I don't see your point, I described the bug precisely.
The capture unmutes the pass-through link to the
sound card, so whatever is captured (white noise),
gets also immediately outputed to the speakers, even
though pulseaudio does not feed that to the sound
card.


As I said before, the white noise bug should be fixed.
With what xawtv versions are you noticing problems? Are you using
xawtv 3.101? If so, xawtv 3.101 assumes that you're using digital

There is nothing to do with xawtv here: as I said,
the noise happens on xorg startup. Starting xawtv
actually makes it to disappear, but I can't always
start xawtv just for that.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html