Re: [pulseaudio-discuss] unused check_required function in modules/alsa/alsa-mixer.c

2011-03-04 Thread Colin Guthrie
'Twas brillig, and David Henningsson at 04/03/11 00:05 did gyre and gimble:
 On 2011-03-03 17:54, Maarten Bosmans wrote:
 I intended to catch diwic on IRC, but didn't see him the last couple
 of days, so in order not to forget, here is my question to the list.

 In this commit some stuff got shuffled around in alsa-mixer
 http://git.0pointer.de/?p=pulseaudio.git;a=commitdiff;h=b0f72311
 With the result that the check_required function is now unused (or
 more precise: only used by itself). This is probably incorrect.

 David, can you explain/fix?
 
 Hmm, in my patch as posted months ago, the call to check_required was
 moved inside the element_probe function only, not moved from
 element_probe to check_required. Colin, can anything have gone wrong
 when you merged it?
 

Hmmm, interesting.

Looking at the upstream commit and the patch file, I have to concur that
yes, something messed up in the application of that patch...

I think it should be fixed by:
 http://git.0pointer.de/?p=pulseaudio.git;a=commitdiff;h=f4a2a8ebf
Can you confirm? I had a look through to see if any other bits had been
misapplied but couldn't see any.


I don't remember that patch (0002) failing git am. The patch after
(0003) did fail, which I mentioned in my reply.

What is more worrying (as I've just replayed what I did), it's
reproducable :s

[colin@jimmy pulseaudio (master)]$ git checkout -b misapply
0ce3017b7407ab1c4094f7ce271bb68319a7eba7
[colin@jimmy pulseaudio (misapply)]$ git am
0002-alsa-mixer-add-required-any-and-required-for-enum-op.patch

Have a look and notice it's been misapplied :s

[colin@jimmy pulseaudio (misapply)]$ git --version
git version 1.7.4.1


Can anyone else spot something dumb on my part before I report this
upstream? It could be a very serious problem.


Col

PS reattached David's patch for convenience.

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]
From ae83e51c82a747332494bf10c245281e49343fe3 Mon Sep 17 00:00:00 2001
From: David Henningsson david.hennings...@canonical.com
Date: Mon, 20 Dec 2010 12:29:27 +0100
Subject: [PATCH 2/6] alsa-mixer: add required-any and required-* for enum options

Now you can add required-any to elements in a path and the path
will be valid as long as at least one of the elements are present.
Also you can have required, required-any and required-absent in
element options, causing a path to be unsupported if an option is
(not) present (simplified example: to skip line in path if
Capture source doesn't have a Line In option).

Signed-off-by: David Henningsson david.hennings...@canonical.com
---
 src/modules/alsa/alsa-mixer.c  |   90 +---
 src/modules/alsa/alsa-mixer.h  |8 ++
 .../alsa/mixer/paths/analog-output.conf.common |5 +
 3 files changed, 91 insertions(+), 12 deletions(-)

diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
index eb50ae2..2c47319 100644
--- a/src/modules/alsa/alsa-mixer.c
+++ b/src/modules/alsa/alsa-mixer.c
@@ -1018,6 +1018,38 @@ static int check_required(pa_alsa_element *e, snd_mixer_elem_t *me) {
 if (e-required_absent == PA_ALSA_REQUIRED_ANY  (has_switch || has_volume || has_enumeration))
 return -1;
 
+if (e-required_any != PA_ALSA_REQUIRED_IGNORE) {
+switch (e-required_any) {
+case PA_ALSA_REQUIRED_VOLUME:
+e-path-req_any_present |= (e-volume_use != PA_ALSA_VOLUME_IGNORE);
+break;
+case PA_ALSA_REQUIRED_SWITCH:
+e-path-req_any_present |= (e-switch_use != PA_ALSA_SWITCH_IGNORE);
+break;
+case PA_ALSA_REQUIRED_ENUMERATION:
+e-path-req_any_present |= (e-enumeration_use != PA_ALSA_ENUMERATION_IGNORE);
+break;
+case PA_ALSA_REQUIRED_ANY:
+e-path-req_any_present |=
+(e-volume_use != PA_ALSA_VOLUME_IGNORE) ||
+(e-switch_use != PA_ALSA_SWITCH_IGNORE) ||
+(e-enumeration_use != PA_ALSA_ENUMERATION_IGNORE);
+break;
+}
+}
+
+if (e-enumeration_use == PA_ALSA_ENUMERATION_SELECT) {
+pa_alsa_option *o;
+PA_LLIST_FOREACH(o, e-options) {
+e-path-req_any_present |= (o-required_any != PA_ALSA_REQUIRED_IGNORE) 
+(o-alsa_idx = 0);
+if (o-required != PA_ALSA_REQUIRED_IGNORE  o-alsa_idx  0)
+return -1;
+if (o-required_absent != PA_ALSA_REQUIRED_IGNORE  o-alsa_idx = 0)
+return -1;
+}
+}
+
 return 0;
 }
 
@@ -1190,9 +1222,6 @@ static int element_probe(pa_alsa_element *e, snd_mixer_t *m) {
 
 }
 
-if (check_required(e, me)  0)
-return -1;
-
 if (e-switch_use == PA_ALSA_SWITCH_SELECT) {
 pa_alsa_option *o;
 

Re: [pulseaudio-discuss] unused check_required function in modules/alsa/alsa-mixer.c

2011-03-04 Thread Colin Guthrie
'Twas brillig, and Colin Guthrie at 04/03/11 10:04 did gyre and gimble:
 Can anyone else spot something dumb on my part before I report this
 upstream? It could be a very serious problem.

Just FYI, I submitted upstream after Arun confirmed the problem on his
system:
http://thread.gmane.org/gmane.comp.version-control.git/168439

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]

___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss


[pulseaudio-discuss] unused check_required function in modules/alsa/alsa-mixer.c

2011-03-03 Thread Maarten Bosmans
I intended to catch diwic on IRC, but didn't see him the last couple
of days, so in order not to forget, here is my question to the list.

In this commit some stuff got shuffled around in alsa-mixer
http://git.0pointer.de/?p=pulseaudio.git;a=commitdiff;h=b0f72311
With the result that the check_required function is now unused (or
more precise: only used by itself). This is probably incorrect.

David, can you explain/fix?

Maarten
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss


Re: [pulseaudio-discuss] unused check_required function in modules/alsa/alsa-mixer.c

2011-03-03 Thread David Henningsson

On 2011-03-03 17:54, Maarten Bosmans wrote:

I intended to catch diwic on IRC, but didn't see him the last couple
of days, so in order not to forget, here is my question to the list.

In this commit some stuff got shuffled around in alsa-mixer
http://git.0pointer.de/?p=pulseaudio.git;a=commitdiff;h=b0f72311
With the result that the check_required function is now unused (or
more precise: only used by itself). This is probably incorrect.

David, can you explain/fix?


Hmm, in my patch as posted months ago, the call to check_required was 
moved inside the element_probe function only, not moved from 
element_probe to check_required. Colin, can anything have gone wrong 
when you merged it?


--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss