* [patch][saa7134] do not change mute state for capturing audio @ 2011-07-10 16:27 Stas Sergeev 2011-07-13 20:53 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-10 16:27 UTC (permalink / raw) To: linux-media; +Cc: Nickolay V. Shmyrev, Mauro Carvalho Chehab [-- Attachment #1: Type: text/plain, Size: 495 bytes --] Hi. 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. :) Signed-off-by: Stas Sergeev <stsp@list.ru> [-- Attachment #2: saa_mute.diff --] [-- Type: text/plain, Size: 1196 bytes --] diff --git a/drivers/media/video/saa7134/saa7134-alsa.c b/drivers/media/video/saa7134/saa7134-alsa.c index 10460fd..d566468 100644 --- a/drivers/media/video/saa7134/saa7134-alsa.c +++ b/drivers/media/video/saa7134/saa7134-alsa.c @@ -77,7 +77,6 @@ typedef struct snd_card_saa7134 { unsigned long iobase; s16 irq; - u16 mute_was_on; spinlock_t lock; } snd_card_saa7134_t; @@ -715,13 +714,6 @@ static int snd_card_saa7134_hw_free(struct snd_pcm_substream * substream) static int snd_card_saa7134_capture_close(struct snd_pcm_substream * substream) { - snd_card_saa7134_t *saa7134 = snd_pcm_substream_chip(substream); - struct saa7134_dev *dev = saa7134->dev; - - if (saa7134->mute_was_on) { - dev->ctl_mute = 1; - saa7134_tvaudio_setmute(dev); - } return 0; } @@ -774,12 +766,6 @@ static int snd_card_saa7134_capture_open(struct snd_pcm_substream * substream) runtime->private_free = snd_card_saa7134_runtime_free; runtime->hw = snd_card_saa7134_capture; - if (dev->ctl_mute != 0) { - saa7134->mute_was_on = 1; - dev->ctl_mute = 0; - saa7134_tvaudio_setmute(dev); - } - err = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS); if (err < 0) ^ permalink raw reply related [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-10 16:27 [patch][saa7134] do not change mute state for capturing audio Stas Sergeev @ 2011-07-13 20:53 ` Mauro Carvalho Chehab 2011-07-13 21:11 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-13 20:53 UTC (permalink / raw) To: Stas Sergeev; +Cc: linux-media, Nickolay V. Shmyrev, Devin Heitmueller Em 10-07-2011 13:27, Stas Sergeev escreveu: > Hi. > > 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. I've pinged Lennart about that and he is suggesting that we should use a non-standard name for the controls, in order to avoid pulseaudio to touch on them. We need first to double check if applications like tvtime and xawtv will work with a different name for the audio volumes. If the existing apps are ok, then maybe all we need to do is to rename all media volumes as something like "<foo> Grabber" or "<foo> V4L". Cheers, Mauro ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-13 20:53 ` Mauro Carvalho Chehab @ 2011-07-13 21:11 ` Stas Sergeev 2011-07-13 22:00 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-13 21:11 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Nickolay V. Shmyrev, Devin Heitmueller 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? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-13 21:11 ` Stas Sergeev @ 2011-07-13 22:00 ` Mauro Carvalho Chehab 2011-07-14 5:39 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-13 22:00 UTC (permalink / raw) To: Stas Sergeev; +Cc: linux-media, Nickolay V. Shmyrev, Devin Heitmueller Em 13-07-2011 18:11, Stas Sergeev escreveu: > 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. Well, on such configuration, there are, in fact, 4 mutes: the two ones you've mentioned, plus the sound card LINE IN mute and the sound card master output. The media drivers should control the input that belongs to saa7134. The userspace applications like pulseaudio should control the sound card volume/mute, but the driver should control the saa7134 mute/audio switch. > 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. > 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? 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. > 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. There's no sense on keeping the device unmuted after stop streaming. > 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. 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 streams. Maybe your board entry is broken for digital streams. > > Can you please name a few downsides of the approach > I proposed? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-13 22:00 ` Mauro Carvalho Chehab @ 2011-07-14 5:39 ` Stas Sergeev 2011-07-15 1:38 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-14 5:39 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Nickolay V. Shmyrev, Devin Heitmueller 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-14 5:39 ` Stas Sergeev @ 2011-07-15 1:38 ` Mauro Carvalho Chehab 2011-07-15 5:40 ` Stas Sergeev ` (2 more replies) 0 siblings, 3 replies; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-15 1:38 UTC (permalink / raw) To: Stas Sergeev; +Cc: linux-media, Nickolay V. Shmyrev, Devin Heitmueller Em 14-07-2011 02:39, Stas Sergeev escreveu: > 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. Huh? The first time, you said it were due to pulseaudio. Then, you said it were due to xawtv, and now you're blaming Xorg startup. Starting X should not be touching on anything, as Xorg itself doesn't have any code to handle an alsa device. Let's reset this thread to the beginning. See bellow. > 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. 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. Detecting that radio is streaming is complex, as the API allows to start stream, while keeping the radio device closed. Anyway, in this case, radio applications need to manually unmute the device. Also, the alsa driver doesn't have any business to do when the audio is wire connected, excepting by providing the mixer controls. 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. In any case, all V4L drivers should have the same behavior on that matter. 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 take care through, as capture start/stop happens at IRQ time. so, it can't sleep. We may likely need to start a workqueue to mute/unmute the driver at capture. We had to do something similar to that at the em28xx, as the DMA start sequence there calls msleep(), and this is not allowed at IRQ time. If you want, feel free to propose a patch fixing that logic at saa7134, instead of just removing it. Cheers, Mauro ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-15 1:38 ` Mauro Carvalho Chehab @ 2011-07-15 5:40 ` Stas Sergeev 2011-07-15 6:16 ` Stas Sergeev 2011-07-17 9:44 ` Stas Sergeev 2 siblings, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2011-07-15 5:40 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Nickolay V. Shmyrev, Devin Heitmueller 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? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-15 1:38 ` Mauro Carvalho Chehab 2011-07-15 5:40 ` Stas Sergeev @ 2011-07-15 6:16 ` Stas Sergeev 2011-07-17 9:44 ` Stas Sergeev 2 siblings, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2011-07-15 6:16 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Nickolay V. Shmyrev, Devin Heitmueller 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-15 1:38 ` Mauro Carvalho Chehab 2011-07-15 5:40 ` Stas Sergeev 2011-07-15 6:16 ` Stas Sergeev @ 2011-07-17 9:44 ` Stas Sergeev 2011-07-17 11:51 ` Mauro Carvalho Chehab 2 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-17 9:44 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-17 9:44 ` Stas Sergeev @ 2011-07-17 11:51 ` Mauro Carvalho Chehab 2011-07-17 12:24 ` Stas Sergeev 2011-07-18 23:16 ` Lennart Poettering 0 siblings, 2 replies; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-17 11:51 UTC (permalink / raw) To: Stas Sergeev Cc: linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel, Lennart Poettering Hi Stas, Em 17-07-2011 06:44, Stas Sergeev escreveu: > 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] (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. I fail to understand why pulseaudio would start capturing sound from a V4L audio at startup. 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 don't know enough about pulseaudio to help on this issue, nor I'm using it currently, so I can't test anything pulsaudio-related. Lennart, Could you please help us with this issue? Thanks! Mauro > --- > > 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-17 11:51 ` Mauro Carvalho Chehab @ 2011-07-17 12:24 ` Stas Sergeev 2011-07-18 23:16 ` Lennart Poettering 1 sibling, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2011-07-17 12:24 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel, Lennart Poettering 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-17 11:51 ` Mauro Carvalho Chehab 2011-07-17 12:24 ` Stas Sergeev @ 2011-07-18 23:16 ` Lennart Poettering 2011-07-19 6:31 ` Stas Sergeev 2011-07-19 13:00 ` Mauro Carvalho Chehab 1 sibling, 2 replies; 64+ messages in thread From: Lennart Poettering @ 2011-07-18 23:16 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Stas Sergeev, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel, lennart Heya, On 17.07.2011 13:51, Mauro Carvalho Chehab wrote: > If pulseaudio is starting sound capture at startup, then it is either > a pulseaudio miss-configuration or a bug there. I fail to understand > why pulseaudio would start capturing sound from a V4L audio at startup. > > 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 don't know enough about pulseaudio to help on this issue, > nor I'm using it currently, so I can't test anything pulsaudio-related. > > Lennart, > > Could you please help us with this issue? 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. Lennart ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-18 23:16 ` Lennart Poettering @ 2011-07-19 6:31 ` Stas Sergeev 2011-07-19 12:25 ` [alsa-devel] " Lennart Poettering 2011-07-19 13:00 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-19 6:31 UTC (permalink / raw) To: Lennart Poettering Cc: Mauro Carvalho Chehab, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel, lennart 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? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [alsa-devel] [patch][saa7134] do not change mute state for capturing audio 2011-07-19 6:31 ` Stas Sergeev @ 2011-07-19 12:25 ` Lennart Poettering 0 siblings, 0 replies; 64+ messages in thread From: Lennart Poettering @ 2011-07-19 12:25 UTC (permalink / raw) To: Stas Sergeev Cc: ALSA devel, Devin Heitmueller, Mauro Carvalho Chehab, Nickolay V. Shmyrev, linux-media On Tue, 19.07.11 10:31, Stas Sergeev (stsp@list.ru) wrote: > 2. "Even if it sucks in general". In this case, what solution > would you propose to get the problem of the white > noise fixed? Well, for removing the probing in PA we'd need a way to reliably figure out in which combinations of input and output we can open a sound card. i.e. we want to know if we can run surround 5.1 output and spdif output at the same time, or surround 5.1 output and stereo input and so on. And we'd need a lot of other attrbites about the sound card, and all that without having to open any PCM device. But that would be really hard to do, the current format neagotiation in ALSA PCM works very differently. And that's the reason why so far nobody has bothered with getting this right. The current code in PA to figure this out is somewhat ugly. It's slow, since we open the card in a lot of different combinations to test what works. It's fragile, since if somebody else has opened the soundcard we get an EBUSY which we take as hint that a specific combination didn't work, and so on. It's a hard problem, Lennart -- Lennart Poettering - Red Hat, Inc. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-18 23:16 ` Lennart Poettering 2011-07-19 6:31 ` Stas Sergeev @ 2011-07-19 13:00 ` Mauro Carvalho Chehab 2011-07-19 13:13 ` [alsa-devel] " Lennart Poettering 2011-07-19 13:49 ` Stas Sergeev 1 sibling, 2 replies; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-19 13:00 UTC (permalink / raw) To: Lennart Poettering Cc: Stas Sergeev, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel, lennart Em 18-07-2011 20:16, Lennart Poettering escreveu: > Heya, > > On 17.07.2011 13:51, Mauro Carvalho Chehab wrote: > >> If pulseaudio is starting sound capture at startup, then it is either >> a pulseaudio miss-configuration or a bug there. I fail to understand >> why pulseaudio would start capturing sound from a V4L audio at startup. >> >> 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 don't know enough about pulseaudio to help on this issue, >> nor I'm using it currently, so I can't test anything pulsaudio-related. >> >> Lennart, >> >> Could you please help us with this issue? > > 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. Lennart, The thing is that starting capture on a video device has some side effects, as it will start capturing from a radio or TV station without specifying the desired frequency. 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. The right thing to do is to get rid of capturing on a video device, if you're not sure that the device is properly tuned. It is easy to detect that an audio device is provided by a v4l device. All you need to do is to look at the parent device via sysfs. Cheers, Mauro ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [alsa-devel] [patch][saa7134] do not change mute state for capturing audio 2011-07-19 13:00 ` Mauro Carvalho Chehab @ 2011-07-19 13:13 ` Lennart Poettering 2011-07-19 13:49 ` Stas Sergeev 1 sibling, 0 replies; 64+ messages in thread From: Lennart Poettering @ 2011-07-19 13:13 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: ALSA devel, Devin Heitmueller, Nickolay V. Shmyrev, Stas Sergeev, linux-media On Tue, 19.07.11 10:00, Mauro Carvalho Chehab (mchehab@infradead.org) wrote: Heya, > The thing is that starting capture on a video device has some side effects, > as it will start capturing from a radio or TV station without specifying > the desired frequency. > > 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. > > The right thing to do is to get rid of capturing on a video device, if you're > not sure that the device is properly tuned. > > It is easy to detect that an audio device is provided by a v4l device. All > you need to do is to look at the parent device via sysfs. So what we actually support in PA, is that you can disable the probing for specific sound cards if you supply a file that describes what should be exposed in PA for the sound card instead. We use that for a number of pro audio cards, where we want to show nicer human readable strings for specific configurations. This is configured in /usr/share/pulseaudio/alsa-mixer/paths/, /usr/share/pulseaudio/alsa-mixer/profile-sets/* and /lib/udev/rules.d/90-pulseaudio.rules. The udev rules files binds a profile set to a specific sound device. The profile set then declares in which combinations a sound card can be opened for input and output, and which mixer paths to expose. Note that the profile sets/mixer paths are supposed to be user-friendly. Hence instead of exposing all options they are designed to expose only the minimum that is useful in the UI. And the emphasis is on usefulness here, so the options the user can choose should be few, not overwhlemingly many. https://tango.0pointer.de/pipermail/pulseaudio-discuss/2009-June/004229.html It might make sense to add that for your TV card to PA as well. Lennart -- Lennart Poettering - Red Hat, Inc. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-19 13:00 ` Mauro Carvalho Chehab 2011-07-19 13:13 ` [alsa-devel] " Lennart Poettering @ 2011-07-19 13:49 ` Stas Sergeev 2011-07-19 14:10 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-19 13:49 UTC (permalink / raw) To: Lennart Poettering Cc: Mauro Carvalho Chehab, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel, lennart 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? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-19 13:49 ` Stas Sergeev @ 2011-07-19 14:10 ` Mauro Carvalho Chehab 2011-07-19 14:56 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-19 14:10 UTC (permalink / raw) To: Stas Sergeev Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel, lennart Em 19-07-2011 10:49, Stas Sergeev escreveu: > 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. It is not something for "a particular driver". All v4l alsa drivers have similar logic: they assume that, if some application is streaming, the device should be unmuted, otherwise capture won't work. > 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. It is not clear, from an application POV, to know what audio pin should be unmuted. Some devices, like, for example, em28xx may have an ac97 connected on it, with lots of muxes on it. For each different video input port, a different mixer set should be used, and the configuration is board-dependent. For example, on Prolink PlayTV USB2, this is wired as: AC97 mono volume => PCM output AC97 master volume => Line out pin AC97 video volume => TV tuner input AC97 line in volume => Svideo or composite input 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. There's no way for an userspace application to know that, since: 1) The device internal connections are not visible on userspace; 2) This is board-dependent. For example, Supercomp USB 2.0 TV has just the opposite setup for TV/svideo: AC97 video volume => Svideo or Composite Input AC97 line in volume => TV Input and doesn't have any output volume connected. 3) On some cases, there are two or even three mutes that need to be changed. Moving such logic to happen at userspace would be very complex, and will break existing applications. Cheers, Mauro. > > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-19 14:10 ` Mauro Carvalho Chehab @ 2011-07-19 14:56 ` Stas Sergeev 2011-07-19 15:27 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-19 14:56 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel 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. :) ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-19 14:56 ` Stas Sergeev @ 2011-07-19 15:27 ` Mauro Carvalho Chehab 2011-07-19 15:50 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-19 15:27 UTC (permalink / raw) To: Stas Sergeev Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel Em 19-07-2011 11:56, Stas Sergeev escreveu: > 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. 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. The business logic coded into the Kernel is that, when audio stream is started or a different input is selected, the driver checks the current applicable mute/volumes and sets the pertinent ones to unmute, muting the others. Yet, they allow users to manually adjust the volume controls, as someone may want for example to use the mixer to mix the original TV audio with a microphone, for example, connected at the LINE IN input that some boards offer. Also, some newer devices are coming with the capability of mixing video inputs (s5p driver recently added a video mixer). It makes sense for applications that use it, to also allow controlling the audio mixer. That's basically why we want to expose such controls to userspace: to allow users to control the audio mixer when they need, for whatever reason. If I understood PA concepts, its philosophy seems to hide those controls that aren't meant to be used by the default usecase. As such, it should not be exposing any mixer/mute at all from a V4L device. The proper fix seems to make PA to use libmedia_dev[1] to detect what audio input devices are associated with a V4L board and removing them for the list of controlled devices by default, eventually allowing the users to add them again via some configuration parameter. [1] http://git.linuxtv.org/v4l-utils.git?a=blob;f=utils/libmedia_dev/README >> 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. :) Xawtv mute/unmute probably needs fix. It uses 3 different API's for that: V4L2, ALSA and OSS. Most of the logic there is for OSS, witch can be removed nowadays. Feel free to submit patches for it. 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. Thanks, Mauro ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-19 15:27 ` Mauro Carvalho Chehab @ 2011-07-19 15:50 ` Stas Sergeev 2011-07-19 18:06 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-19 15:50 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-19 15:50 ` Stas Sergeev @ 2011-07-19 18:06 ` Mauro Carvalho Chehab 2011-07-19 18:38 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-19 18:06 UTC (permalink / raw) To: Stas Sergeev Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel Em 19-07-2011 12:50, Stas Sergeev escreveu: > 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. No, you're mistaken: on most boards, you have only one volume control/switch, for capture. So, it would be a "master capture switch", 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. > 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. > And, if it is made exported, what will still prevent us > from dropping the auto-unmute stuff? 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. It should be noticed that the alsa module is only enabled on devices that provides PCM output via the PCI or USB bus. On the other hand, the V4L2_CID_AUDIO_MUTE control is available for all devices that are capable of muting/unmuting the audio. That's why V4L applications rely on it, instead of using alsa mixers. I dunno what's your specific saa7134 device model, but, from what I understood, instead of using the PCM output, you're using the LINE OUT pin. 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. >> 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. We need a solution that works for both simple and complex devices. - [1] IMHO, "LINE OUT" pin doesn't fit very well on playback/capture concept. The capture volume/switch controls the A/D circuits for capture, while the playblack controls the D/A circuits. However, the LINE OUT pin gets audio from the captured data, and doesn't allow to direct any other PCM data into the D/A. So, it is not a "complete" playback type of control. So, it won't fit at the "Master Playback Switch" type. Cheers, Mauro ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-19 18:06 ` Mauro Carvalho Chehab @ 2011-07-19 18:38 ` Stas Sergeev 2011-07-19 19:29 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-19 18:38 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel 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. :( ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-19 18:38 ` Stas Sergeev @ 2011-07-19 19:29 ` Mauro Carvalho Chehab 2011-07-19 21:57 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-19 19:29 UTC (permalink / raw) To: Stas Sergeev Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel Em 19-07-2011 15:38, Stas Sergeev escreveu: > 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. Adding a new volume control that changes the mute values for the other controls or renaming it don't solve anything. >> 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. That would be wrong. >>> 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? I fail to see why this would be a problem. The problem I see is that PA is trying to handle a V4L device as if it would be a normal audio capture pin, and starting a capture while the device is not ready for that, as no input or TV/radio station were selected at the time PA starts capturing. >> 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. :( 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. I2S is for PCM (as it is a digital audio interface). The other flag is for analog. So, if the device is a saa7134, it is easy to split the analog output and the PCM one. For saa7133 and saa7135, you probably need to double check at the datasheet, is is there a way to disable/enable just the I2S interface, but, from saa7134_enable_i2s(): /* Start I2S */ saa_writeb(SAA7134_I2S_AUDIO_OUTPUT, 0x11); I bet that there is some value (maybe 0?) that disables I2S transfers, muting the PCM stream. It should be tested if saa7133/5 devices accept to enable/disable PCM streams if the device is already streaming. Cheers, Mauro ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-19 19:29 ` Mauro Carvalho Chehab @ 2011-07-19 21:57 ` Stas Sergeev 2011-07-20 0:55 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-19 21:57 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-19 21:57 ` Stas Sergeev @ 2011-07-20 0:55 ` Mauro Carvalho Chehab 2011-07-20 5:28 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-20 0:55 UTC (permalink / raw) To: Stas Sergeev Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel Em 19-07-2011 18:57, Stas Sergeev escreveu: > 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, Any app can do it right now via the V4L2 api. > 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? PA is doing the wrong thing here, due to the lack of a better API support: it is starting stream on a device as a hacky way of probing it. As Lennart pointed, even considering a pure audio device, this is ugly and takes time. IMO, the right long term fix is to provide alsa some ioctl that allows PA to get the needed info without needing to start streaming, and, for the short term, to prevent it to start capture on tuner/grabber devices. > 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. >>> 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... Maybe your device is not a saa7134. For saa7133/saa7135, the mute/unmute seems to be done via GPIO, and via amux. > 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). As I said before, before adding this patch upstream, we need to double check if it will work with saa7134, saa7133 and saa7135, preserving the old behavior on those devices. - saa7134: Don't touch at the analog mute at the alsa driver Instead of setting both analog and digital parts of the driver, alsa just needs to enable/disable the I2S capture. Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> diff --git a/drivers/media/video/saa7134/saa7134-alsa.c b/drivers/media/video/saa7134/saa7134-alsa.c index 10460fd..2edcdd2 100644 --- a/drivers/media/video/saa7134/saa7134-alsa.c +++ b/drivers/media/video/saa7134/saa7134-alsa.c @@ -720,7 +720,7 @@ static int snd_card_saa7134_capture_close(struct snd_pcm_substream * substream) if (saa7134->mute_was_on) { dev->ctl_mute = 1; - saa7134_tvaudio_setmute(dev); + saa7134_i2s_mute(dev, dev->ctl_mute); } return 0; } @@ -777,7 +777,7 @@ static int snd_card_saa7134_capture_open(struct snd_pcm_substream * substream) if (dev->ctl_mute != 0) { saa7134->mute_was_on = 1; dev->ctl_mute = 0; - saa7134_tvaudio_setmute(dev); + saa7134_i2s_mute(dev, dev->ctl_mute); } err = snd_pcm_hw_constraint_integer(runtime, diff --git a/drivers/media/video/saa7134/saa7134-tvaudio.c b/drivers/media/video/saa7134/saa7134-tvaudio.c index 57e646b..9cc81ed 100644 --- a/drivers/media/video/saa7134/saa7134-tvaudio.c +++ b/drivers/media/video/saa7134/saa7134-tvaudio.c @@ -184,6 +184,15 @@ static void tvaudio_setcarrier(struct saa7134_dev *dev, #define SAA7134_MUTE_ANALOG 0x04 #define SAA7134_MUTE_I2S 0x40 +void saa7134_i2s_mute(struct saa7134_dev *dev, unsigned int mute) +{ + if (PCI_DEVICE_ID_PHILIPS_SAA7134 == dev->pci->device) + saa_andorb(SAA7134_AUDIO_FORMAT_CTRL, SAA7134_MUTE_I2S, + mute ? SAA7134_MUTE_I2S : 0); + else + saa_writeb(SAA7134_I2S_AUDIO_OUTPUT, mute ? 0x11 : 0); +} + static void mute_input_7134(struct saa7134_dev *dev) { unsigned int mute; @@ -220,10 +229,11 @@ static void mute_input_7134(struct saa7134_dev *dev) /* 7134 mute */ saa_writeb(SAA7134_AUDIO_MUTE_CTRL, mute ? SAA7134_MUTE_MASK | - SAA7134_MUTE_ANALOG | - SAA7134_MUTE_I2S : + SAA7134_MUTE_ANALOG : SAA7134_MUTE_MASK); + saa7134_i2s_mute(dev, mute); + /* switch internal audio mux */ switch (in->amux) { case TV: ausel=0xc0; ics=0x00; ocs=0x02; break; diff --git a/drivers/media/video/saa7134/saa7134.h b/drivers/media/video/saa7134/saa7134.h index bc8d6bb..7b104cc 100644 --- a/drivers/media/video/saa7134/saa7134.h +++ b/drivers/media/video/saa7134/saa7134.h @@ -821,6 +821,7 @@ int saa7134_tvaudio_do_scan(struct saa7134_dev *dev); int saa_dsp_writel(struct saa7134_dev *dev, int reg, u32 value); void saa7134_enable_i2s(struct saa7134_dev *dev); +void saa7134_i2s_mute(struct saa7134_dev *dev, unsigned int mute); /* ----------------------------------------------------------- */ /* saa7134-oss.c */ ^ permalink raw reply related [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-20 0:55 ` Mauro Carvalho Chehab @ 2011-07-20 5:28 ` Stas Sergeev 2011-07-20 10:32 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-20 5:28 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel 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! ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-20 5:28 ` Stas Sergeev @ 2011-07-20 10:32 ` Mauro Carvalho Chehab 2011-07-20 10:41 ` Mauro Carvalho Chehab 2011-07-20 10:45 ` Stas Sergeev 0 siblings, 2 replies; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-20 10:32 UTC (permalink / raw) To: Stas Sergeev Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel Em 20-07-2011 02:28, Stas Sergeev escreveu: > 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? I won't keep discussing something that won't be merged, as it will cause regressions. >>> 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. 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. >> The enclosed patch probably does the trick (completely untested). > I'll be able to test it on avertv 307 the next week-end. The patch is not that simple. The driver needs to set the device inputs accordingly, otherwise it will break support for digital audio. In the specific case of avertv 307, this patch alone may not work. I suspect that there is a problem at the GPIO settings for mute on its board entry: [SAA7134_BOARD_AVERMEDIA_STUDIO_307] = { ... .inputs = {{ .name = name_tv, .vmux = 1, .amux = TV, .tv = 1, .gpio = 0x00, },{ ... .mute = { .name = name_mute, .amux = LINE1, .gpio = 0x00, }, See: mute GPIO's are equal for TV GPIO. That means that it will select the TV gpio for "mute". - [PATCHv2 - BROKEN] saa7134: Don't touch at the analog mute at the alsa driver Via the alsa driver, it is possible to start capturing from an audio input. When capture is started, the driver will unmute the audio input associated with the selected video input, if it were muted. However, if the device is using a wire for the audio output, it may produce audio at the speakers. This patch changes the mute logic to: 1) on saa7134, don't touch at the ANALOG_MUTE at alsa unmute call; 2) don't change the GPIO's. 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. Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> diff --git a/drivers/media/video/saa7134/saa7134-alsa.c b/drivers/media/video/saa7134/saa7134-alsa.c index 10460fd..cbc665a 100644 --- a/drivers/media/video/saa7134/saa7134-alsa.c +++ b/drivers/media/video/saa7134/saa7134-alsa.c @@ -77,7 +77,6 @@ typedef struct snd_card_saa7134 { unsigned long iobase; s16 irq; - u16 mute_was_on; spinlock_t lock; } snd_card_saa7134_t; @@ -718,9 +717,10 @@ static int snd_card_saa7134_capture_close(struct snd_pcm_substream * substream) snd_card_saa7134_t *saa7134 = snd_pcm_substream_chip(substream); struct saa7134_dev *dev = saa7134->dev; - if (saa7134->mute_was_on) { + if (dev->mute_was_on) { dev->ctl_mute = 1; saa7134_tvaudio_setmute(dev); + dev->mute_was_on = false; } return 0; } @@ -775,7 +775,7 @@ static int snd_card_saa7134_capture_open(struct snd_pcm_substream * substream) runtime->hw = snd_card_saa7134_capture; if (dev->ctl_mute != 0) { - saa7134->mute_was_on = 1; + dev->mute_was_on = true; dev->ctl_mute = 0; saa7134_tvaudio_setmute(dev); } diff --git a/drivers/media/video/saa7134/saa7134-tvaudio.c b/drivers/media/video/saa7134/saa7134-tvaudio.c index 57e646b..535aa75 100644 --- a/drivers/media/video/saa7134/saa7134-tvaudio.c +++ b/drivers/media/video/saa7134/saa7134-tvaudio.c @@ -190,6 +190,9 @@ static void mute_input_7134(struct saa7134_dev *dev) struct saa7134_input *in; int ausel=0, ics=0, ocs=0; int mask; + bool change_analog_mute; + + change_analog_mute = dev->mute_was_on ? false : true; /* look what is to do ... */ in = dev->input; @@ -204,7 +207,8 @@ static void mute_input_7134(struct saa7134_dev *dev) in = &card(dev).mute; } - if (dev->hw_mute == mute && + + if (dev->hw_mute == mute && !dev->mute_was_on && dev->hw_input == in && !dev->insuspend) { dprintk("mute/input: nothing to do [mute=%d,input=%s]\n", mute,in->name); @@ -216,13 +220,18 @@ static void mute_input_7134(struct saa7134_dev *dev) dev->hw_mute = mute; dev->hw_input = in; - if (PCI_DEVICE_ID_PHILIPS_SAA7134 == dev->pci->device) + if (PCI_DEVICE_ID_PHILIPS_SAA7134 == dev->pci->device) { + u32 mask = ~0; + u32 mute_val = SAA7134_MUTE_MASK; + + if (!change_analog_mute) + mask ^= SAA7134_MUTE_ANALOG; + if (mute) + mute_val |= SAA7134_MUTE_I2S | SAA7134_MUTE_ANALOG; + /* 7134 mute */ - saa_writeb(SAA7134_AUDIO_MUTE_CTRL, mute ? - SAA7134_MUTE_MASK | - SAA7134_MUTE_ANALOG | - SAA7134_MUTE_I2S : - SAA7134_MUTE_MASK); + saa_andorb(SAA7134_AUDIO_MUTE_CTRL, mask, mute_val); + } /* switch internal audio mux */ switch (in->amux) { @@ -241,7 +250,7 @@ static void mute_input_7134(struct saa7134_dev *dev) saa_andorb(SAA7134_SIF_SAMPLE_FREQ, 0x03, 0x01); /* switch gpio-connected external audio mux */ - if (0 == card(dev).gpiomask) + if (0 == card(dev).gpiomask || !change_analog_mute) return; mask = card(dev).gpiomask; @@ -725,6 +734,9 @@ static int mute_input_7133(struct saa7134_dev *dev) u32 xbarin, xbarout; int mask; struct saa7134_input *in; + bool change_analog_mute; + + change_analog_mute = dev->mute_was_on ? false : true; xbarin = 0x03; switch (dev->input->amux) { @@ -750,9 +762,8 @@ static int mute_input_7133(struct saa7134_dev *dev) saa_writel(0x594 >> 2, reg); - /* switch gpio-connected external audio mux */ - if (0 != card(dev).gpiomask) { + if (0 != card(dev).gpiomask && change_analog_mute) { mask = card(dev).gpiomask; if (card(dev).mute.name && dev->ctl_mute) diff --git a/drivers/media/video/saa7134/saa7134.h b/drivers/media/video/saa7134/saa7134.h index bc8d6bb..ae34f68 100644 --- a/drivers/media/video/saa7134/saa7134.h +++ b/drivers/media/video/saa7134/saa7134.h @@ -602,6 +602,7 @@ struct saa7134_dev { int ctl_saturation; int ctl_freq; int ctl_mute; /* audio */ + bool mute_was_on; int ctl_volume; int ctl_invert; /* private */ int ctl_mirror; ^ permalink raw reply related [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-20 10:32 ` Mauro Carvalho Chehab @ 2011-07-20 10:41 ` Mauro Carvalho Chehab 2011-07-20 10:45 ` Stas Sergeev 1 sibling, 0 replies; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-20 10:41 UTC (permalink / raw) To: Stas Sergeev Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel Em 20-07-2011 07:32, Mauro Carvalho Chehab escreveu: > Em 20-07-2011 02:28, Stas Sergeev escreveu: >> 20.07.2011 04:55, Mauro Carvalho Chehab wrote: > [PATCHv2 - BROKEN] saa7134: Don't touch at the analog mute at the alsa driver > > Via the alsa driver, it is possible to start capturing from an audio input. > When capture is started, the driver will unmute the audio input associated > with the selected video input, if it were muted. > > However, if the device is using a wire for the audio output, it may produce > audio at the speakers. This patch changes the mute logic to: > 1) on saa7134, don't touch at the ANALOG_MUTE at alsa unmute call; > 2) don't change the GPIO's. > > 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. [PATCHv3] saa7134: Don't touch at the analog mute at the alsa driver Via the alsa driver, it is possible to start capturing from an audio input. When capture is started, the driver will unmute the audio input associated with the selected video input, if it were muted. However, if the device is using a wire for the audio output, it may produce audio at the speakers. This patch changes the mute logic to don't touch at the ANALOG_MUTE at alsa unmute call, for saa7134. Not sure if this will produce any effect, as it will depend on how the board is wired, but it is a worth trial. Patch is untested. Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> diff --git a/drivers/media/video/saa7134/saa7134-alsa.c b/drivers/media/video/saa7134/saa7134-alsa.c index 10460fd..cbc665a 100644 --- a/drivers/media/video/saa7134/saa7134-alsa.c +++ b/drivers/media/video/saa7134/saa7134-alsa.c @@ -77,7 +77,6 @@ typedef struct snd_card_saa7134 { unsigned long iobase; s16 irq; - u16 mute_was_on; spinlock_t lock; } snd_card_saa7134_t; @@ -718,9 +717,10 @@ static int snd_card_saa7134_capture_close(struct snd_pcm_substream * substream) snd_card_saa7134_t *saa7134 = snd_pcm_substream_chip(substream); struct saa7134_dev *dev = saa7134->dev; - if (saa7134->mute_was_on) { + if (dev->mute_was_on) { dev->ctl_mute = 1; saa7134_tvaudio_setmute(dev); + dev->mute_was_on = false; } return 0; } @@ -775,7 +775,7 @@ static int snd_card_saa7134_capture_open(struct snd_pcm_substream * substream) runtime->hw = snd_card_saa7134_capture; if (dev->ctl_mute != 0) { - saa7134->mute_was_on = 1; + dev->mute_was_on = true; dev->ctl_mute = 0; saa7134_tvaudio_setmute(dev); } diff --git a/drivers/media/video/saa7134/saa7134-tvaudio.c b/drivers/media/video/saa7134/saa7134-tvaudio.c index 57e646b..11631f4 100644 --- a/drivers/media/video/saa7134/saa7134-tvaudio.c +++ b/drivers/media/video/saa7134/saa7134-tvaudio.c @@ -190,6 +190,9 @@ static void mute_input_7134(struct saa7134_dev *dev) struct saa7134_input *in; int ausel=0, ics=0, ocs=0; int mask; + bool change_analog_mute; + + change_analog_mute = dev->mute_was_on ? false : true; /* look what is to do ... */ in = dev->input; @@ -204,7 +207,8 @@ static void mute_input_7134(struct saa7134_dev *dev) in = &card(dev).mute; } - if (dev->hw_mute == mute && + + if (dev->hw_mute == mute && !dev->mute_was_on && dev->hw_input == in && !dev->insuspend) { dprintk("mute/input: nothing to do [mute=%d,input=%s]\n", mute,in->name); @@ -216,13 +220,18 @@ static void mute_input_7134(struct saa7134_dev *dev) dev->hw_mute = mute; dev->hw_input = in; - if (PCI_DEVICE_ID_PHILIPS_SAA7134 == dev->pci->device) + if (PCI_DEVICE_ID_PHILIPS_SAA7134 == dev->pci->device) { + u32 mask = ~0; + u32 mute_val = SAA7134_MUTE_MASK; + + if (!change_analog_mute) + mask ^= SAA7134_MUTE_ANALOG; + if (mute) + mute_val |= SAA7134_MUTE_I2S | SAA7134_MUTE_ANALOG; + /* 7134 mute */ - saa_writeb(SAA7134_AUDIO_MUTE_CTRL, mute ? - SAA7134_MUTE_MASK | - SAA7134_MUTE_ANALOG | - SAA7134_MUTE_I2S : - SAA7134_MUTE_MASK); + saa_andorb(SAA7134_AUDIO_MUTE_CTRL, mask, mute_val); + } /* switch internal audio mux */ switch (in->amux) { diff --git a/drivers/media/video/saa7134/saa7134.h b/drivers/media/video/saa7134/saa7134.h index bc8d6bb..ae34f68 100644 --- a/drivers/media/video/saa7134/saa7134.h +++ b/drivers/media/video/saa7134/saa7134.h @@ -602,6 +602,7 @@ struct saa7134_dev { int ctl_saturation; int ctl_freq; int ctl_mute; /* audio */ + bool mute_was_on; int ctl_volume; int ctl_invert; /* private */ int ctl_mirror; ^ permalink raw reply related [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-20 10:32 ` Mauro Carvalho Chehab 2011-07-20 10:41 ` Mauro Carvalho Chehab @ 2011-07-20 10:45 ` Stas Sergeev 2011-07-20 10:48 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-20 10:45 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-20 10:45 ` Stas Sergeev @ 2011-07-20 10:48 ` Mauro Carvalho Chehab 2011-07-20 10:55 ` Stas Sergeev 2011-07-22 7:51 ` Stas Sergeev 0 siblings, 2 replies; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-20 10:48 UTC (permalink / raw) To: Stas Sergeev Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel Em 20-07-2011 07:45, Stas Sergeev escreveu: > 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'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. > >> 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-20 10:48 ` Mauro Carvalho Chehab @ 2011-07-20 10:55 ` Stas Sergeev 2011-07-22 7:51 ` Stas Sergeev 1 sibling, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2011-07-20 10:55 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Lennart Poettering, linux-media, Nickolay V. Shmyrev, Devin Heitmueller, ALSA devel 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! :)) ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-20 10:48 ` Mauro Carvalho Chehab 2011-07-20 10:55 ` Stas Sergeev @ 2011-07-22 7:51 ` Stas Sergeev 2011-07-22 12:28 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-22 7:51 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media 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. :) ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-22 7:51 ` Stas Sergeev @ 2011-07-22 12:28 ` Mauro Carvalho Chehab 2011-07-22 12:39 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-22 12:28 UTC (permalink / raw) To: Stas Sergeev; +Cc: linux-media Em 22-07-2011 04:51, Stas Sergeev escreveu: > Ok, Mauro, so may I take your silence as an evidence > that this reiterating myth about the mplayer breakage > is just a myth? It is due to my lack of time of explaining the obvious for you. Changing the behavior of the kernel drivers has consequences on userspace, called regressions. Regressions are not allowed at the Linux Kernel: a binary that used to run with an old kernel should keep running on a new kernel. 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. > 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. :) I probably won't push my own patch, at least for now, as it is not tested, and I'm currently lacking time to install a few saa7134 boards for testing. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-22 12:28 ` Mauro Carvalho Chehab @ 2011-07-22 12:39 ` Stas Sergeev 2011-07-22 12:49 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-22 12:39 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media 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? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-22 12:39 ` Stas Sergeev @ 2011-07-22 12:49 ` Mauro Carvalho Chehab 2011-07-22 12:56 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-22 12:49 UTC (permalink / raw) To: Stas Sergeev; +Cc: linux-media Em 22-07-2011 09:39, Stas Sergeev escreveu: > 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. 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 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? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-22 12:49 ` Mauro Carvalho Chehab @ 2011-07-22 12:56 ` Stas Sergeev 2011-07-22 13:03 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-22 12:56 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media 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? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-22 12:56 ` Stas Sergeev @ 2011-07-22 13:03 ` Mauro Carvalho Chehab 2011-07-22 20:40 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-22 13:03 UTC (permalink / raw) To: Stas Sergeev; +Cc: linux-media Em 22-07-2011 09:56, Stas Sergeev escreveu: > 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? 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" ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-22 13:03 ` Mauro Carvalho Chehab @ 2011-07-22 20:40 ` Stas Sergeev 2011-07-23 1:28 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-22 20:40 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media 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? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-22 20:40 ` Stas Sergeev @ 2011-07-23 1:28 ` Mauro Carvalho Chehab 2011-07-23 7:44 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-23 1:28 UTC (permalink / raw) To: Stas Sergeev; +Cc: linux-media Em 22-07-2011 17:40, Stas Sergeev escreveu: > 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? Mplayer was just one example of an application that I know it doesn't call V4L2_CID_AUDIO_MUTE to unmute. I didn't check for other applications and scripts that may break with such change. 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). You can't associate such logic to any ioctl, due to the same reasons. Also, associating with V4L open also will cause side effects, as udev opens all V4L devices when the device is detected. You should also remind that it is possible to use a separate application (like v4l2-ctl) while a device is opened by other applications, and even to not have the device opened while streaming (radio applications allow that). Mauro. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-23 1:28 ` Mauro Carvalho Chehab @ 2011-07-23 7:44 ` Stas Sergeev 2011-07-23 13:06 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-23 7:44 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media 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? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-23 7:44 ` Stas Sergeev @ 2011-07-23 13:06 ` Mauro Carvalho Chehab 2011-07-23 13:20 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-23 13:06 UTC (permalink / raw) To: Stas Sergeev; +Cc: linux-media Em 23-07-2011 04:44, Stas Sergeev escreveu: > 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. If application needs to change due to a patch, this is a regression, as it will break binary compatibility with non-patched versions. NACK. >> 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? Even if it mutes, every time a channel is changed, it will be unmuted, if you put such unmute logic at VIDIOC_S_FREQUENCY. Mauro. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-23 13:06 ` Mauro Carvalho Chehab @ 2011-07-23 13:20 ` Stas Sergeev 2011-07-23 15:09 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-23 13:20 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-23 13:20 ` Stas Sergeev @ 2011-07-23 15:09 ` Mauro Carvalho Chehab 2011-07-23 15:14 ` Stas Sergeev ` (2 more replies) 0 siblings, 3 replies; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-23 15:09 UTC (permalink / raw) To: Stas Sergeev; +Cc: linux-media Em 23-07-2011 10:20, Stas Sergeev escreveu: > 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. Hard to tell about your solution without seeing a patch. Not sure if this will be consistent, especially if PA restarts for whatever reason (X restart? manual restart?). Anyway, we're discussing a lot for a kernel fix for PA, while the right thing to do is to fix PA itself. Mauro. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-23 15:09 ` Mauro Carvalho Chehab @ 2011-07-23 15:14 ` Stas Sergeev 2011-07-23 15:25 ` Stas Sergeev 2011-07-24 17:45 ` Stas Sergeev 2 siblings, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2011-07-23 15:14 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-23 15:09 ` Mauro Carvalho Chehab 2011-07-23 15:14 ` Stas Sergeev @ 2011-07-23 15:25 ` Stas Sergeev 2011-07-24 17:45 ` Stas Sergeev 2 siblings, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2011-07-23 15:25 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-23 15:09 ` Mauro Carvalho Chehab 2011-07-23 15:14 ` Stas Sergeev 2011-07-23 15:25 ` Stas Sergeev @ 2011-07-24 17:45 ` Stas Sergeev 2011-07-24 18:36 ` Mauro Carvalho Chehab 2 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-07-24 17:45 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Nickolay V. Shmyrev 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... ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-24 17:45 ` Stas Sergeev @ 2011-07-24 18:36 ` Mauro Carvalho Chehab 2011-07-24 19:00 ` Stas Sergeev ` (2 more replies) 0 siblings, 3 replies; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-24 18:36 UTC (permalink / raw) To: Stas Sergeev Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel Em 24-07-2011 14:45, Stas Sergeev escreveu: > 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. The automute code works fine. Maybe you have a strong interference at the default tuning frequency, leading into saa7134 miss-detection. For saa7134 driver: 1) There is an audio carrier; 2) an application wants to listen audio; 3) the device is at automute mode. So, when there's an audio carrier, it will unmute the audio at streaming start. The driver is doing the _right_ thing by letting the audio to flow to PA, when it starts the capture. Unfortunately, on your specific device, starting audio capture also enables audio at the audio output pin. So, you're noticing a problem. People with a saa7134 device without alsa stream won't notice it (old devices). People with alsa stream without anything connected to the LINE OUT people aren't noticing it, if PA is not copying the saa7134 PCM IN stream to the sound card PCM out device (the default, for PA). 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. > 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. > Anyway, as long as the automute code is broken, > we should either start fixing it, or fix PA, or fix mplayer... As I always said, the fix should be at PA, as it makes no sense to start capturing at saa7134 without first configuring it. So, or PA should be converted into a V4L2-aware application, in order to properly init the device (with seems to be the wrong approach) or it _SHOULD_NOT_ automatically enable capture on those devices, as this may cause undesired side effects, on the devices that have the capture pin directly wired to the output pin, witch seems to be the case of your device. PS.: it seems that you've removed Lennart/alsa people from the c/c. I'm re-adding them, as I'm expecting a fix from their side. > Dunno. I wonder how come so many bugs left unfixed > for so long, resulting in a white noise to people... 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. Mauro. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-24 18:36 ` Mauro Carvalho Chehab @ 2011-07-24 19:00 ` Stas Sergeev 2011-07-25 11:15 ` Stas Sergeev 2011-09-18 15:18 ` Stas Sergeev 2 siblings, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2011-07-24 19:00 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-24 18:36 ` Mauro Carvalho Chehab 2011-07-24 19:00 ` Stas Sergeev @ 2011-07-25 11:15 ` Stas Sergeev 2011-09-18 15:18 ` Stas Sergeev 2 siblings, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2011-07-25 11:15 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-07-24 18:36 ` Mauro Carvalho Chehab 2011-07-24 19:00 ` Stas Sergeev 2011-07-25 11:15 ` Stas Sergeev @ 2011-09-18 15:18 ` Stas Sergeev 2011-09-24 10:57 ` Mauro Carvalho Chehab 2011-09-24 12:05 ` [patch][saa7134] do not change mute state " Mauro Carvalho Chehab 2 siblings, 2 replies; 64+ messages in thread From: Stas Sergeev @ 2011-09-18 15:18 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel [-- Attachment #1: Type: text/plain, Size: 374 bytes --] 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? [-- Attachment #2: 0001-saa7134-fix-automute.patch --] [-- Type: text/x-patch, Size: 2072 bytes --] >From ccdfa126e98b5484f4a08de591ac8d89f775251c Mon Sep 17 00:00:00 2001 From: Stas Sergeev <stsp@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 [-- Attachment #3: 0002-saa7134-introduce-std-module-parameter-to-force-vide.patch --] [-- Type: text/x-patch, Size: 3462 bytes --] >From 70709f12f7161c98cb7ebae104b520dc30c6bd53 Mon Sep 17 00:00:00 2001 From: Stas Sergeev <stsp@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) { + strcat(buf, "-"); + strcat(buf, secam); + if (strncasecmp(buf, tvnorms[i].name, + strlen(buf)) != 0) + enable = 0; + } + } + tvnorms[i].enabled = enable; + if (enable && idx == -1) + idx = i; + } + if (idx == -1) + return -EINVAL; /* init video hw */ - set_tvnorm(dev,&tvnorms[0]); + set_tvnorm(dev,&tvnorms[idx]); video_mux(dev,0); saa7134_tvaudio_setmute(dev); saa7134_tvaudio_setvolume(dev,dev->ctl_volume); diff --git a/drivers/media/video/saa7134/saa7134.h b/drivers/media/video/saa7134/saa7134.h index 28eb103..a0eddce 100644 --- a/drivers/media/video/saa7134/saa7134.h +++ b/drivers/media/video/saa7134/saa7134.h @@ -77,6 +77,7 @@ enum saa7134_video_out { struct saa7134_tvnorm { char *name; v4l2_std_id id; + int enabled; /* video decoder */ unsigned int sync_control; -- 1.7.6 ^ permalink raw reply related [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-18 15:18 ` Stas Sergeev @ 2011-09-24 10:57 ` Mauro Carvalho Chehab 2011-09-24 11:12 ` Stas Sergeev 2011-09-24 12:05 ` [patch][saa7134] do not change mute state " Mauro Carvalho Chehab 1 sibling, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-09-24 10:57 UTC (permalink / raw) To: Stas Sergeev Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel Em 18-09-2011 12:18, Stas Sergeev escreveu: > 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? Please, one patch per email. Patchwork (or any kernel maintainer script) won't catch more than one patch per email. See: http://patchwork.linuxtv.org/patch/7862/ As those are the two last patches marked as new at patchwork, I've manually uploaded it as two separate emails, in order to allow me to queue them: http://patchwork.linuxtv.org/patch/7940/ http://patchwork.linuxtv.org/patch/7941/ 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. 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. Regards, Mauro ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-24 10:57 ` Mauro Carvalho Chehab @ 2011-09-24 11:12 ` Stas Sergeev 2011-09-24 12:12 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-09-24 11:12 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-24 11:12 ` Stas Sergeev @ 2011-09-24 12:12 ` Mauro Carvalho Chehab 2011-09-24 12:36 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-09-24 12:12 UTC (permalink / raw) To: Stas Sergeev Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel Em 24-09-2011 08:12, Stas Sergeev escreveu: > 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. The scan audio logic only enables multiple audio standard detection if the userspace application tells it to do. The right fix here is to fix the application. The secam hack is due to a problem related to Secam L and Secam L'. > >> 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. The right fix that pulseaudio should not touch at the audio mixers for the video boards. Not all boards have an audio carrier detection like saa7134. Regards, Mauro. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-24 12:12 ` Mauro Carvalho Chehab @ 2011-09-24 12:36 ` Stas Sergeev 2011-09-24 12:48 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-09-24 12:36 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel 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? ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-24 12:36 ` Stas Sergeev @ 2011-09-24 12:48 ` Mauro Carvalho Chehab 2011-09-24 13:20 ` Stas Sergeev ` (2 more replies) 0 siblings, 3 replies; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-09-24 12:48 UTC (permalink / raw) To: Stas Sergeev Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel Em 24-09-2011 09:36, Stas Sergeev escreveu: > 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? A first scan at driver's init can be removed, IMO. The thing is that newer versions of udev will open the device, to do a VIDIOC_QUERYCAP. Not sure if this will wake up the tvaudio kthread to do a scan. Regards, Mauro ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-24 12:48 ` Mauro Carvalho Chehab @ 2011-09-24 13:20 ` Stas Sergeev 2011-09-24 15:09 ` Mauro Carvalho Chehab 2011-12-03 20:40 ` Stas Sergeev 2012-01-02 10:12 ` [resend][patch][saa7134] fix automute " Stas Sergeev 2 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-09-24 13:20 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-24 13:20 ` Stas Sergeev @ 2011-09-24 15:09 ` Mauro Carvalho Chehab 2011-09-24 15:51 ` Stas Sergeev 0 siblings, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-09-24 15:09 UTC (permalink / raw) To: Stas Sergeev Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel Em 24-09-2011 10:20, Stas Sergeev escreveu: > 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. OK. > >> 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. Yes. >> 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. >> 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. 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. I'm not sure if it is safe to make mplayer to use the audio mixer. It is probably a good idea doing that, as it will also work fine with webcams that provide alsa inputs. > 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-24 15:09 ` Mauro Carvalho Chehab @ 2011-09-24 15:51 ` Stas Sergeev 0 siblings, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2011-09-24 15:51 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel 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. :) ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-24 12:48 ` Mauro Carvalho Chehab 2011-09-24 13:20 ` Stas Sergeev @ 2011-12-03 20:40 ` Stas Sergeev 2012-01-02 10:12 ` [resend][patch][saa7134] fix automute " Stas Sergeev 2 siblings, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2011-12-03 20:40 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel [-- Attachment #1: Type: text/plain, Size: 376 bytes --] 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 <stsp@users.sourceforge.net> [-- Attachment #2: 0001-saa7134-fix-automute-logic.patch --] [-- Type: text/plain, Size: 5922 bytes --] From d8c8a05449b06ee7599e7c7d9e8aaeaa07d0fadb Mon Sep 17 00:00:00 2001 From: Stas Sergeev <stsp@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) 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 +1024,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 */ @@ -1029,13 +1034,19 @@ int saa7134_tvaudio_init2(struct saa7134_dev *dev) dev->name); /* XXX: missing error handling here */ } - saa7134_tvaudio_do_scan(dev); } saa7134_enable_i2s(dev); return 0; } +int saa7134_tvaudio_close(struct saa7134_dev *dev) +{ + dev->automute = 1; + /* anything else to undo? */ + return 0; +} + int saa7134_tvaudio_fini(struct saa7134_dev *dev) { /* shutdown tvaudio thread */ diff --git a/drivers/media/video/saa7134/saa7134-video.c b/drivers/media/video/saa7134/saa7134-video.c index 9cf7914f..417034e 100644 --- a/drivers/media/video/saa7134/saa7134-video.c +++ b/drivers/media/video/saa7134/saa7134-video.c @@ -1462,6 +1462,8 @@ static int video_release(struct file *file) struct saa6588_command cmd; unsigned long flags; + saa7134_tvaudio_close(dev); + /* turn off overlay */ if (res_check(fh, RESOURCE_OVERLAY)) { spin_lock_irqsave(&dev->slock,flags); diff --git a/drivers/media/video/saa7134/saa7134.h b/drivers/media/video/saa7134/saa7134.h index 9b55068..621b1ed 100644 --- a/drivers/media/video/saa7134/saa7134.h +++ b/drivers/media/video/saa7134/saa7134.h @@ -817,6 +817,7 @@ void saa7134_tvaudio_init(struct saa7134_dev *dev); int saa7134_tvaudio_init2(struct saa7134_dev *dev); int saa7134_tvaudio_fini(struct saa7134_dev *dev); int saa7134_tvaudio_do_scan(struct saa7134_dev *dev); +int saa7134_tvaudio_close(struct saa7134_dev *dev); int saa_dsp_writel(struct saa7134_dev *dev, int reg, u32 value); -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 64+ messages in thread
* [resend][patch][saa7134] fix automute for capturing audio 2011-09-24 12:48 ` Mauro Carvalho Chehab 2011-09-24 13:20 ` Stas Sergeev 2011-12-03 20:40 ` Stas Sergeev @ 2012-01-02 10:12 ` Stas Sergeev 2 siblings, 0 replies; 64+ messages in thread From: Stas Sergeev @ 2012-01-02 10:12 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media [-- Attachment #1: Type: text/plain, Size: 422 bytes --] Mauro, are you available to take a look into patch? 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 <stsp@users.sourceforge.net> [-- Attachment #2: 0001-saa7134-fix-automute-logic.patch --] [-- Type: text/plain, Size: 5922 bytes --] From d8c8a05449b06ee7599e7c7d9e8aaeaa07d0fadb Mon Sep 17 00:00:00 2001 From: Stas Sergeev <stsp@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) 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 +1024,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 */ @@ -1029,13 +1034,19 @@ int saa7134_tvaudio_init2(struct saa7134_dev *dev) dev->name); /* XXX: missing error handling here */ } - saa7134_tvaudio_do_scan(dev); } saa7134_enable_i2s(dev); return 0; } +int saa7134_tvaudio_close(struct saa7134_dev *dev) +{ + dev->automute = 1; + /* anything else to undo? */ + return 0; +} + int saa7134_tvaudio_fini(struct saa7134_dev *dev) { /* shutdown tvaudio thread */ diff --git a/drivers/media/video/saa7134/saa7134-video.c b/drivers/media/video/saa7134/saa7134-video.c index 9cf7914f..417034e 100644 --- a/drivers/media/video/saa7134/saa7134-video.c +++ b/drivers/media/video/saa7134/saa7134-video.c @@ -1462,6 +1462,8 @@ static int video_release(struct file *file) struct saa6588_command cmd; unsigned long flags; + saa7134_tvaudio_close(dev); + /* turn off overlay */ if (res_check(fh, RESOURCE_OVERLAY)) { spin_lock_irqsave(&dev->slock,flags); diff --git a/drivers/media/video/saa7134/saa7134.h b/drivers/media/video/saa7134/saa7134.h index 9b55068..621b1ed 100644 --- a/drivers/media/video/saa7134/saa7134.h +++ b/drivers/media/video/saa7134/saa7134.h @@ -817,6 +817,7 @@ void saa7134_tvaudio_init(struct saa7134_dev *dev); int saa7134_tvaudio_init2(struct saa7134_dev *dev); int saa7134_tvaudio_fini(struct saa7134_dev *dev); int saa7134_tvaudio_do_scan(struct saa7134_dev *dev); +int saa7134_tvaudio_close(struct saa7134_dev *dev); int saa_dsp_writel(struct saa7134_dev *dev, int reg, u32 value); -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-18 15:18 ` Stas Sergeev 2011-09-24 10:57 ` Mauro Carvalho Chehab @ 2011-09-24 12:05 ` Mauro Carvalho Chehab 2011-09-24 12:33 ` Stas Sergeev 1 sibling, 1 reply; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-09-24 12:05 UTC (permalink / raw) To: Stas Sergeev Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel Em 18-09-2011 12:18, Stas Sergeev escreveu: > 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? > > Content-Type: text/plain; charset="utf-8" > MIME-Version: 1.0 > Content-Transfer-Encoding: 7bit > Subject: [1/2,saa7134] do not change mute state for capturing audio > Date: Sun, 18 Sep 2011 14:18:34 -0000 > From: Stas Sergeev <stsp@list.ru> > X-Patchwork-Id: 7940 > Message-Id: <4E760BCA.6080900-patch1@list.ru> > To: Mauro Carvalho Chehab <mchehab@infradead.org> > Cc: linux-media@vger.kernel.org, "Nickolay V. Shmyrev" <nshmyrev@yandex.ru>, > Lennart Poettering <lpoetter@redhat.com>, > ALSA devel <alsa-devel@alsa-project.org> > > 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 <stsp@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) { 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)) { + 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 +354,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 +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? If the carrier is good, this should be enough: dev->automute = 0; > > } 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 */ 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? Thanks, Mauro ^ permalink raw reply related [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-24 12:05 ` [patch][saa7134] do not change mute state " Mauro Carvalho Chehab @ 2011-09-24 12:33 ` Stas Sergeev 2011-09-24 12:46 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 64+ messages in thread From: Stas Sergeev @ 2011-09-24 12:33 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel 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. ^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [patch][saa7134] do not change mute state for capturing audio 2011-09-24 12:33 ` Stas Sergeev @ 2011-09-24 12:46 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 64+ messages in thread From: Mauro Carvalho Chehab @ 2011-09-24 12:46 UTC (permalink / raw) To: Stas Sergeev Cc: linux-media, Nickolay V. Shmyrev, Lennart Poettering, ALSA devel Em 24-09-2011 09:33, Stas Sergeev escreveu: > 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? Yes. patch were of course not tested ;) I meant to say: if (!(dev->tvnorm->id & scan->std)) { > >>> @@ -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. 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. > 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. This is a dirty hack, and it assumes that the first scan should be discarded. This is true only on certain environments. If someone is using the board on an environment without udev and pulseaudio, this trick will break the first tuning. >> 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. 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. Regards, Mauro. ^ permalink raw reply [flat|nested] 64+ messages in thread
end of thread, other threads:[~2012-01-02 10:13 UTC | newest] Thread overview: 64+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-07-10 16:27 [patch][saa7134] do not change mute state for capturing audio Stas Sergeev 2011-07-13 20:53 ` Mauro Carvalho Chehab 2011-07-13 21:11 ` Stas Sergeev 2011-07-13 22:00 ` Mauro Carvalho Chehab 2011-07-14 5:39 ` Stas Sergeev 2011-07-15 1:38 ` Mauro Carvalho Chehab 2011-07-15 5:40 ` Stas Sergeev 2011-07-15 6:16 ` Stas Sergeev 2011-07-17 9:44 ` Stas Sergeev 2011-07-17 11:51 ` Mauro Carvalho Chehab 2011-07-17 12:24 ` Stas Sergeev 2011-07-18 23:16 ` Lennart Poettering 2011-07-19 6:31 ` Stas Sergeev 2011-07-19 12:25 ` [alsa-devel] " Lennart Poettering 2011-07-19 13:00 ` Mauro Carvalho Chehab 2011-07-19 13:13 ` [alsa-devel] " Lennart Poettering 2011-07-19 13:49 ` Stas Sergeev 2011-07-19 14:10 ` Mauro Carvalho Chehab 2011-07-19 14:56 ` Stas Sergeev 2011-07-19 15:27 ` Mauro Carvalho Chehab 2011-07-19 15:50 ` Stas Sergeev 2011-07-19 18:06 ` Mauro Carvalho Chehab 2011-07-19 18:38 ` Stas Sergeev 2011-07-19 19:29 ` Mauro Carvalho Chehab 2011-07-19 21:57 ` Stas Sergeev 2011-07-20 0:55 ` Mauro Carvalho Chehab 2011-07-20 5:28 ` Stas Sergeev 2011-07-20 10:32 ` Mauro Carvalho Chehab 2011-07-20 10:41 ` Mauro Carvalho Chehab 2011-07-20 10:45 ` Stas Sergeev 2011-07-20 10:48 ` Mauro Carvalho Chehab 2011-07-20 10:55 ` Stas Sergeev 2011-07-22 7:51 ` Stas Sergeev 2011-07-22 12:28 ` Mauro Carvalho Chehab 2011-07-22 12:39 ` Stas Sergeev 2011-07-22 12:49 ` Mauro Carvalho Chehab 2011-07-22 12:56 ` Stas Sergeev 2011-07-22 13:03 ` Mauro Carvalho Chehab 2011-07-22 20:40 ` Stas Sergeev 2011-07-23 1:28 ` Mauro Carvalho Chehab 2011-07-23 7:44 ` Stas Sergeev 2011-07-23 13:06 ` Mauro Carvalho Chehab 2011-07-23 13:20 ` Stas Sergeev 2011-07-23 15:09 ` Mauro Carvalho Chehab 2011-07-23 15:14 ` Stas Sergeev 2011-07-23 15:25 ` Stas Sergeev 2011-07-24 17:45 ` Stas Sergeev 2011-07-24 18:36 ` Mauro Carvalho Chehab 2011-07-24 19:00 ` Stas Sergeev 2011-07-25 11:15 ` Stas Sergeev 2011-09-18 15:18 ` Stas Sergeev 2011-09-24 10:57 ` Mauro Carvalho Chehab 2011-09-24 11:12 ` Stas Sergeev 2011-09-24 12:12 ` Mauro Carvalho Chehab 2011-09-24 12:36 ` Stas Sergeev 2011-09-24 12:48 ` Mauro Carvalho Chehab 2011-09-24 13:20 ` Stas Sergeev 2011-09-24 15:09 ` Mauro Carvalho Chehab 2011-09-24 15:51 ` Stas Sergeev 2011-12-03 20:40 ` Stas Sergeev 2012-01-02 10:12 ` [resend][patch][saa7134] fix automute " Stas Sergeev 2011-09-24 12:05 ` [patch][saa7134] do not change mute state " Mauro Carvalho Chehab 2011-09-24 12:33 ` Stas Sergeev 2011-09-24 12:46 ` Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).