Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-20 Thread Arun Raghavan 
On Wed, 2011-10-19 at 13:11 +0300, Tanu Kaskinen wrote:
 On Wed, 2011-10-19 at 13:00 +0300, Colin Guthrie wrote:
  'Twas brillig, and David Henningsson at 18/10/11 20:56 did gyre and gimble:
   While in general I agree that a boolean is a fine success/failure return
   type, I think in Pulseaudio the convention is to stick just to ints.
   
   Hmm. A quick 'grep -r return TRUE' of PulseAudio source tree seems to
   give enough results to falsify that assumption.
 
 That gives a big list indeed. But did you check the context? I went
 through quite a lot (but not nearly all) of the output from 'git grep -n
 -e return TRUE -e return FALSE' and the overwhelming majority of the
 cases were from functions of which whole purpose is to give a yes/no
 answer, or where the function may or may not have a side effect (ie.
 where a nothing happened case is not an error). There were some cases
 where return FALSE really meant that an error happened, but most of
 those were from the recent format stuff...
 
  We're quite happy to return bools on internal stuff. It's just when
  dealing with client-site/public APIs that we stick to ints.
  
  So as this is an internal function, I think it's fine.
 
 It's not about having a portable ABI, it's only about the convention
 that errors are reported as negative integers. But if nobody else cares,
 then I'm fine too with using bools.

I usually only code with a negative value return on failure if the
callee can/should distinguish between different types of errors.

-- Arun

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-20 Thread Arun Raghavan 
On Wed, 2011-10-12 at 12:40 +0200, David Henningsson wrote:
 Note: There is still no notification when status availability changes.
 
 Signed-off-by: David Henningsson david.hennings...@canonical.com
 ---
[...]
 diff --git a/PROTOCOL b/PROTOCOL
 index 8c69190..b8b61e4 100644
 --- a/PROTOCOL
 +++ b/PROTOCOL
 @@ -278,6 +278,13 @@ New field in PA_COMMAND_UNDERFLOW:
  
  int64_t index
  
 +## v24, implemented by = 2.0
 +
 +New field in all commands that send/receive port introspection data
 +(PA_COMMAND_GET_(SOURCE|SINK)_INFO, PA_COMMAND_GET_(SOURCE|SINK)_INFO_LIST]):
 +
 +uint32_t available
 +

Is there a reason for this to be larger than uint8_t?

   If you just changed the protocol, read this
  ## module-tunnel depends on the sink/source/sink-input/source-input protocol
  ## internals, so if you changed these, you might have broken module-tunnel.

Does module-tunnel need to care about any of this?

[...]
 [diff --git a/src/pulse/def.h b/src/pulse/def.h
 index f43e864..8a74fad 100644
 --- a/src/pulse/def.h
 +++ b/src/pulse/def.h
 @@ -967,6 +967,21 @@ typedef void (*pa_free_cb_t)(void *p);
   * playback, \since 1.0 */
  #define PA_STREAM_EVENT_FORMAT_LOST format-lost
  
 +/** Port availability / jack detection status
 + * \since 2.0 */
 +typedef enum pa_port_available {
 +PA_PORT_AVAILABLE_UNKNOWN = 0, /** This port does not support jack 
 detection \since 2.0 */
 +PA_PORT_AVAILABLE_NO = 1,  /** This port is not available, likely 
 because the jack is not plugged in. \since 2.0 */
 +PA_PORT_AVAILABLE_YES = 2, /** This port is available, likely 
 because the jack is plugged in. \since 2.0 */
 +} pa_port_available_t;
 +
 +/** \cond fulldocs */
 +#define PA_PORT_AVAILABLE_UNKNOWN PA_PORT_AVAILABLE_UNKNOWN
 +#define PA_PORT_AVAILABLE_NO PA_PORT_AVAILABLE_NO
 +#define PA_PORT_AVAILABLE_YES PA_PORT_AVAILABLE_YES

I'd drop the likely because since that's going to depend on other bits
of implementation (for example a jack maybe unavailable on the current
profile and your comment will only be true if there's a policy module to
switch profiles on jack insertion).

 diff --git a/src/pulse/introspect.h b/src/pulse/introspect.h
 index 1d77d45..7732736 100644
 --- a/src/pulse/introspect.h
 +++ b/src/pulse/introspect.h
 @@ -202,6 +202,7 @@ typedef struct pa_sink_port_info {
  const char *name;   /** Name of this port */
  const char *description;/** Description of this port */
  uint32_t priority;  /** The higher this value is the 
 more useful this port is as a default */
 +pa_port_available_t available;  /** PA_PORT_AVAILABLE_UNKNOWN, 
 PA_PORT_AVAILABLE_NO or PA_PORT_AVAILABLE_YES \since 2.0 */
  } pa_sink_port_info;
  
  /** Stores information about sinks. Please note that this structure
 @@ -281,6 +282,7 @@ typedef struct pa_source_port_info {
  const char *name;   /** Name of this port */
  const char *description;/** Description of this port */
  uint32_t priority;  /** The higher this value is the 
 more useful this port is as a default */
 +pa_port_available_t available;  /** PA_PORT_AVAILABLE_UNKNOWN, 
 PA_PORT_AVAILABLE_NO or PA_PORT_AVAILABLE_YES \since 2.0 */
  } pa_source_port_info;

I think the docs already link to the enum type, so the long comment
might be replaced by Whether the port is available or not or something
similar.

[...]
 diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
 index 7f639e2..39ca117 100644
 --- a/src/pulsecore/sink.h
 +++ b/src/pulsecore/sink.h
 @@ -60,6 +60,7 @@ struct pa_device_port {
  char *description;
  
  unsigned priority;
 +pa_port_available_t available; /** PA_PORT_AVAILABLE_UNKNOWN, 
 PA_PORT_AVAILABLE_NO or PA_PORT_AVAILABLE_YES \since 2.0 */
  
  /* .. followed by some implementation specific data */
  };

I think someone already mentioned this doesn't need to be a doxygen
comment.

-- Arun

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-20 Thread David Henningsson 

Thanks for the review.

On 10/20/2011 10:25 AM, Arun Raghavan wrote:

On Wed, 2011-10-12 at 12:40 +0200, David Henningsson wrote:

Note: There is still no notification when status availability changes.

Signed-off-by: David Henningssondavid.hennings...@canonical.com
---

[...]

diff --git a/PROTOCOL b/PROTOCOL
index 8c69190..b8b61e4 100644
--- a/PROTOCOL
+++ b/PROTOCOL
@@ -278,6 +278,13 @@ New field in PA_COMMAND_UNDERFLOW:

  int64_t index

+## v24, implemented by= 2.0
+
+New field in all commands that send/receive port introspection data
+(PA_COMMAND_GET_(SOURCE|SINK)_INFO, PA_COMMAND_GET_(SOURCE|SINK)_INFO_LIST]):
+
+uint32_t available
+


Is there a reason for this to be larger than uint8_t?


Not at this point, but diverging from Ubuntu 11.10's implementation 
would not resolve the protocol skew.



   If you just changed the protocol, read this
  ## module-tunnel depends on the sink/source/sink-input/source-input protocol
  ## internals, so if you changed these, you might have broken module-tunnel.


Does module-tunnel need to care about any of this?


Yes, did you miss that part of the patch?


[...]

[diff --git a/src/pulse/def.h b/src/pulse/def.h
index f43e864..8a74fad 100644
--- a/src/pulse/def.h
+++ b/src/pulse/def.h
@@ -967,6 +967,21 @@ typedef void (*pa_free_cb_t)(void *p);
   * playback, \since 1.0 */
  #define PA_STREAM_EVENT_FORMAT_LOST format-lost

+/** Port availability / jack detection status
+ * \since 2.0 */
+typedef enum pa_port_available {
+PA_PORT_AVAILABLE_UNKNOWN = 0, /**  This port does not support jack 
detection \since 2.0 */
+PA_PORT_AVAILABLE_NO = 1,  /**  This port is not available, likely 
because the jack is not plugged in. \since 2.0 */
+PA_PORT_AVAILABLE_YES = 2, /**  This port is available, likely 
because the jack is plugged in. \since 2.0 */
+} pa_port_available_t;
+
+/** \cond fulldocs */
+#define PA_PORT_AVAILABLE_UNKNOWN PA_PORT_AVAILABLE_UNKNOWN
+#define PA_PORT_AVAILABLE_NO PA_PORT_AVAILABLE_NO
+#define PA_PORT_AVAILABLE_YES PA_PORT_AVAILABLE_YES


I'd drop the likely because since that's going to depend on other bits
of implementation (for example a jack maybe unavailable on the current
profile


The port available status is independent from whether a profile is 
active or not, and also, a port can belong to several profiles (in 
patches soon to come).



and your comment will only be true if there's a policy module to
switch profiles on jack insertion).


No, it will be true if there is a port implementation that sets jack 
availability status.





diff --git a/src/pulse/introspect.h b/src/pulse/introspect.h
index 1d77d45..7732736 100644
--- a/src/pulse/introspect.h
+++ b/src/pulse/introspect.h
@@ -202,6 +202,7 @@ typedef struct pa_sink_port_info {
  const char *name;   /**  Name of this port */
  const char *description;/**  Description of this port */
  uint32_t priority;  /**  The higher this value is the 
more useful this port is as a default */
+pa_port_available_t available;  /**  PA_PORT_AVAILABLE_UNKNOWN, 
PA_PORT_AVAILABLE_NO or PA_PORT_AVAILABLE_YES \since 2.0 */
  } pa_sink_port_info;

  /** Stores information about sinks. Please note that this structure
@@ -281,6 +282,7 @@ typedef struct pa_source_port_info {
  const char *name;   /**  Name of this port */
  const char *description;/**  Description of this port */
  uint32_t priority;  /**  The higher this value is the 
more useful this port is as a default */
+pa_port_available_t available;  /**  PA_PORT_AVAILABLE_UNKNOWN, 
PA_PORT_AVAILABLE_NO or PA_PORT_AVAILABLE_YES \since 2.0 */
  } pa_source_port_info;


I think the docs already link to the enum type, so the long comment
might be replaced by Whether the port is available or not or something
similar.


Ok, want me to resend with the comment changed?

Also, for the bool vs int thing, I think we all agree on that bools are 
okay in this context.




[...]

diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index 7f639e2..39ca117 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -60,6 +60,7 @@ struct pa_device_port {
  char *description;

  unsigned priority;
+pa_port_available_t available; /**  PA_PORT_AVAILABLE_UNKNOWN, 
PA_PORT_AVAILABLE_NO or PA_PORT_AVAILABLE_YES \since 2.0 */

  /* .. followed by some implementation specific data */
  };


I think someone already mentioned this doesn't need to be a doxygen
comment.


This is already fixed in latest posted version of patch (which, for 
reference, is here 
http://lists.freedesktop.org/archives/pulseaudio-discuss/2011-October/011833.html 
)



--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-19 Thread Colin Guthrie 
'Twas brillig, and David Henningsson at 18/10/11 20:56 did gyre and gimble:
 While in general I agree that a boolean is a fine success/failure return
 type, I think in Pulseaudio the convention is to stick just to ints.
 
 Hmm. A quick 'grep -r return TRUE' of PulseAudio source tree seems to
 give enough results to falsify that assumption.

We're quite happy to return bools on internal stuff. It's just when
dealing with client-site/public APIs that we stick to ints.

So as this is an internal function, I think it's fine.

Col

-- 

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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-19 Thread Colin Guthrie 
'Twas brillig, and David Henningsson at 19/10/11 05:55 did gyre and gimble:
 On 10/19/2011 12:11 AM, Pierre-Louis Bossart wrote:
 Did you miss my previous explanation, or did you find it insufficient?
 I'm repeating it below:

 The protocol skew in Ubuntu 11.10 was actually a mistake on my part.
 Since the UI changes that would depend on this information being
 available was backed out, I probably should have backed the actual
 protocol change out as well.

 Anyway, here is the patch that forms one of the base features for jack
 detection, and brings upstream out of protocol skew with Ubuntu 11.10.

 I honestly did not understand the explanation, most likely because I use
 Fedora for historical reasons and have no idea about the content of
 Ubuntu
 11.10...What type of client will make use of this information?
 
 My use case is volume control UIs (pavucontrol, gnome volume control,
 etc) - they can use the information to hide three of the four HDMI
 ports, keeping only the right one. Or hide headphones when they are not
 plugged in.
 
 But in Ubuntu 11.10 no client actually uses this information, which is
 why the protocol skew was a mistake.

It'll still be needed at some point anyway, so I don't think the skew is
a big problem. We'll make sure this is in the first patch that pokes
protocol versions.

But yes, as David said the availability info can be used to indicate
whether a port is available or not (while not in this patch it will be
in further patch series, and David is posting this bit of it now to get
the protocol bump in now to avoid the problems we had with Meego
protocol deviations).

In Windows 7 for example this is quite clearly labelled as Available vs.
Unavailable in text and by desaturating the icons.


I would say that to see an example of the GUI in win7 to look at my
slides from the Desktop Summit (slide 4)

https://www.desktopsummit.org/program/sessions/pulseaudio-control-and-command-state-desktop-integration-gnome-kde

But sadly the PDF version doesn't handle transitions to the overlays
really don't work :(

So I've attached the relevant screencap here.

Col



-- 

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/
attachment: Sound2.PNG___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-19 Thread Tanu Kaskinen 
On Wed, 2011-10-19 at 13:00 +0300, Colin Guthrie wrote:
 'Twas brillig, and David Henningsson at 18/10/11 20:56 did gyre and gimble:
  While in general I agree that a boolean is a fine success/failure return
  type, I think in Pulseaudio the convention is to stick just to ints.
  
  Hmm. A quick 'grep -r return TRUE' of PulseAudio source tree seems to
  give enough results to falsify that assumption.

That gives a big list indeed. But did you check the context? I went
through quite a lot (but not nearly all) of the output from 'git grep -n
-e return TRUE -e return FALSE' and the overwhelming majority of the
cases were from functions of which whole purpose is to give a yes/no
answer, or where the function may or may not have a side effect (ie.
where a nothing happened case is not an error). There were some cases
where return FALSE really meant that an error happened, but most of
those were from the recent format stuff...

 We're quite happy to return bools on internal stuff. It's just when
 dealing with client-site/public APIs that we stick to ints.
 
 So as this is an internal function, I think it's fine.

It's not about having a portable ABI, it's only about the convention
that errors are reported as negative integers. But if nobody else cares,
then I'm fine too with using bools.

-- 
Tanu

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-19 Thread Tanu Kaskinen 
On Wed, 2011-10-19 at 13:11 +0300, Tanu Kaskinen wrote:
 That gives a big list indeed. But did you check the context? I went
 through quite a lot (but not nearly all) of the output from 'git grep -n
 -e return TRUE -e return FALSE' and the overwhelming majority of the

I actually meant

git grep -B20 -n -e return TRUE -e return FALSE

(note the -B20 option that was missing in my previous mail)

-- 
Tanu

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-18 Thread Tanu Kaskinen 
Hi,

Looks quite good to me. Some review comments below.

On Wed, 2011-10-12 at 12:40 +0200, David Henningsson wrote:
 Note: There is still no notification when status availability changes.
 
 Signed-off-by: David Henningsson david.hennings...@canonical.com
 ---
 
 The protocol skew in Ubuntu 11.10 was actually a mistake on my part. Since 
 the UI
 changes that would depend on this information being available was backed out,
 I probably should have backed the actual protocol change out as well. 
 
 Anyway, here is the patch that forms one of the base features for jack 
 detection,
 and brings upstream out of protocol skew with Ubuntu 11.10.
 
 (Reposted: Thanks courmisch for finding a typo.)
 
  PROTOCOL|7 +++
  configure.ac|2 +-
  src/modules/module-tunnel.c |   91 
 +--
  src/pulse/def.h |   15 ++
  src/pulse/introspect.c  |   16 +++
  src/pulse/introspect.h  |2 +
  src/pulsecore/protocol-native.c |4 ++
  src/pulsecore/sink.h|1 +
  8 files changed, 85 insertions(+), 53 deletions(-)
 
 diff --git a/PROTOCOL b/PROTOCOL
 index 8c69190..b8b61e4 100644
 --- a/PROTOCOL
 +++ b/PROTOCOL
 @@ -278,6 +278,13 @@ New field in PA_COMMAND_UNDERFLOW:
  
  int64_t index
  
 +## v24, implemented by = 2.0
 +
 +New field in all commands that send/receive port introspection data
 +(PA_COMMAND_GET_(SOURCE|SINK)_INFO, PA_COMMAND_GET_(SOURCE|SINK)_INFO_LIST]):
 +
 +uint32_t available
 +
   If you just changed the protocol, read this
  ## module-tunnel depends on the sink/source/sink-input/source-input protocol
  ## internals, so if you changed these, you might have broken module-tunnel.
 diff --git a/configure.ac b/configure.ac
 index 0bf40a8..d57dbc5 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -36,7 +36,7 @@ AC_SUBST(PA_MINOR, pa_minor)
  AC_SUBST(PA_MAJORMINOR, pa_major.pa_minor)
  
  AC_SUBST(PA_API_VERSION, 12)
 -AC_SUBST(PA_PROTOCOL_VERSION, 23)
 +AC_SUBST(PA_PROTOCOL_VERSION, 24)
  
  # The stable ABI for client applications, for the version info x:y:z
  # always will hold y=z
 diff --git a/src/modules/module-tunnel.c b/src/modules/module-tunnel.c
 index faee995..0422c87 100644
 --- a/src/modules/module-tunnel.c
 +++ b/src/modules/module-tunnel.c
 @@ -996,6 +996,41 @@ fail:
  pa_module_unload_request(u-module, TRUE);
  }
  
 +static pa_bool_t read_ports(struct userdata *u, pa_tagstruct *t)

While in general I agree that a boolean is a fine success/failure return
type, I think in Pulseaudio the convention is to stick just to ints.

 +{
 +if (u-version = 16) {
 +uint32_t n_ports;
 +const char *s;
 +
 +if (pa_tagstruct_getu32(t, n_ports)) {
 +pa_log(Parse failure);

I would prefer a bit more informative error message (Parse failure
while reading the number of ports.). Oh, now I noticed that this is
just old code moving to a new place. Never mind...

 +return FALSE;
 +}
 +
 +for (uint32_t j = 0; j  n_ports; j++) {
 +uint32_t priority;
 +
 +if (pa_tagstruct_gets(t, s)  0 || /* name */
 +pa_tagstruct_gets(t, s)  0 || /* description */
 +pa_tagstruct_getu32(t, priority)  0) {
 +
 +pa_log(Parse failure);
 +return FALSE;
 +}
 +if (u-version = 24  pa_tagstruct_getu32(t, priority)  0) { 
 /* available */
 +pa_log(Parse failure);
 +return FALSE;
 +}
 +}
 +
 +if (pa_tagstruct_gets(t, s)  0) { /* active port */
 +pa_log(Parse failure);
 +return FALSE;
 +}
 +}
 +return TRUE;
 +}
 +
  #ifdef TUNNEL_SINK
  
  /* Called from main context */
 @@ -1066,32 +1101,8 @@ static void sink_info_cb(pa_pdispatch *pd, uint32_t 
 command,  uint32_t tag, pa_t
  }
  }
  
 -if (u-version = 16) {
 -uint32_t n_ports;
 -const char *s;
 -
 -if (pa_tagstruct_getu32(t, n_ports)) {
 -pa_log(Parse failure);
 -goto fail;
 -}
 -
 -for (uint32_t j = 0; j  n_ports; j++) {
 -uint32_t priority;
 -
 -if (pa_tagstruct_gets(t, s)  0 || /* name */
 -pa_tagstruct_gets(t, s)  0 || /* description */
 -pa_tagstruct_getu32(t, priority)  0) {
 -
 -pa_log(Parse failure);
 -goto fail;
 -}
 -}
 -
 -if (pa_tagstruct_gets(t, s)  0) { /* active port */
 -pa_log(Parse failure);
 -goto fail;
 -}
 -}
 +if (!read_ports(u, t))
 +goto fail;
  
  if (u-version = 21) {
  uint8_t n_formats;
 @@ -1318,32 +1329,8 @@ static void source_info_cb(pa_pdispatch *pd, uint32_t 
 command,  uint32_t tag, pa
  }
  }
  
 -if (u-version = 16) {
 -uint32_t n_ports;
 -const

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-18 Thread David Henningsson 

On 10/18/2011 07:16 PM, Tanu Kaskinen wrote:

Hi,

Looks quite good to me. Some review comments below.


Thanks for the review.


On Wed, 2011-10-12 at 12:40 +0200, David Henningsson wrote:

Note: There is still no notification when status availability changes.

Signed-off-by: David Henningssondavid.hennings...@canonical.com
---

The protocol skew in Ubuntu 11.10 was actually a mistake on my part. Since the 
UI
changes that would depend on this information being available was backed out,
I probably should have backed the actual protocol change out as well.

Anyway, here is the patch that forms one of the base features for jack 
detection,
and brings upstream out of protocol skew with Ubuntu 11.10.

(Reposted: Thanks courmisch for finding a typo.)

  PROTOCOL|7 +++
  configure.ac|2 +-
  src/modules/module-tunnel.c |   91 +--
  src/pulse/def.h |   15 ++
  src/pulse/introspect.c  |   16 +++
  src/pulse/introspect.h  |2 +
  src/pulsecore/protocol-native.c |4 ++
  src/pulsecore/sink.h|1 +
  8 files changed, 85 insertions(+), 53 deletions(-)

diff --git a/PROTOCOL b/PROTOCOL
index 8c69190..b8b61e4 100644
--- a/PROTOCOL
+++ b/PROTOCOL
@@ -278,6 +278,13 @@ New field in PA_COMMAND_UNDERFLOW:

  int64_t index

+## v24, implemented by= 2.0
+
+New field in all commands that send/receive port introspection data
+(PA_COMMAND_GET_(SOURCE|SINK)_INFO, PA_COMMAND_GET_(SOURCE|SINK)_INFO_LIST]):
+
+uint32_t available
+
   If you just changed the protocol, read this
  ## module-tunnel depends on the sink/source/sink-input/source-input protocol
  ## internals, so if you changed these, you might have broken module-tunnel.
diff --git a/configure.ac b/configure.ac
index 0bf40a8..d57dbc5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,7 +36,7 @@ AC_SUBST(PA_MINOR, pa_minor)
  AC_SUBST(PA_MAJORMINOR, pa_major.pa_minor)

  AC_SUBST(PA_API_VERSION, 12)
-AC_SUBST(PA_PROTOCOL_VERSION, 23)
+AC_SUBST(PA_PROTOCOL_VERSION, 24)

  # The stable ABI for client applications, for the version info x:y:z
  # always will hold y=z
diff --git a/src/modules/module-tunnel.c b/src/modules/module-tunnel.c
index faee995..0422c87 100644
--- a/src/modules/module-tunnel.c
+++ b/src/modules/module-tunnel.c
@@ -996,6 +996,41 @@ fail:
  pa_module_unload_request(u-module, TRUE);
  }

+static pa_bool_t read_ports(struct userdata *u, pa_tagstruct *t)


While in general I agree that a boolean is a fine success/failure return
type, I think in Pulseaudio the convention is to stick just to ints.


Hmm. A quick 'grep -r return TRUE' of PulseAudio source tree seems to 
give enough results to falsify that assumption.



+{
+if (u-version= 16) {
+uint32_t n_ports;
+const char *s;
+
+if (pa_tagstruct_getu32(t,n_ports)) {
+pa_log(Parse failure);


I would prefer a bit more informative error message (Parse failure
while reading the number of ports.). Oh, now I noticed that this is
just old code moving to a new place. Never mind...


+return FALSE;
+}
+
+for (uint32_t j = 0; j  n_ports; j++) {
+uint32_t priority;
+
+if (pa_tagstruct_gets(t,s)  0 || /* name */
+pa_tagstruct_gets(t,s)  0 || /* description */
+pa_tagstruct_getu32(t,priority)  0) {
+
+pa_log(Parse failure);
+return FALSE;
+}
+if (u-version= 24  pa_tagstruct_getu32(t,priority)  0) { /* 
available */
+pa_log(Parse failure);
+return FALSE;
+}
+}
+
+if (pa_tagstruct_gets(t,s)  0) { /* active port */
+pa_log(Parse failure);
+return FALSE;
+}
+}
+return TRUE;
+}
+
  #ifdef TUNNEL_SINK

  /* Called from main context */
@@ -1066,32 +1101,8 @@ static void sink_info_cb(pa_pdispatch *pd, uint32_t 
command,  uint32_t tag, pa_t
  }
  }

-if (u-version= 16) {
-uint32_t n_ports;
-const char *s;
-
-if (pa_tagstruct_getu32(t,n_ports)) {
-pa_log(Parse failure);
-goto fail;
-}
-
-for (uint32_t j = 0; j  n_ports; j++) {
-uint32_t priority;
-
-if (pa_tagstruct_gets(t,s)  0 || /* name */
-pa_tagstruct_gets(t,s)  0 || /* description */
-pa_tagstruct_getu32(t,priority)  0) {
-
-pa_log(Parse failure);
-goto fail;
-}
-}
-
-if (pa_tagstruct_gets(t,s)  0) { /* active port */
-pa_log(Parse failure);
-goto fail;
-}
-}
+if (!read_ports(u, t))
+goto fail;

  if (u-version= 21) {
  uint8_t n_formats;
@@ -1318,32 +1329,8 @@ static void source_info_cb(pa_pdispatch *pd, uint32_t 
command,  uint32_t tag, pa
  }
  }

-if (u-version=

[pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-18 Thread David Henningsson 
Note: There is still no notification when status availability changes.

Signed-off-by: David Henningsson david.hennings...@canonical.com
---
 PROTOCOL|   10 
 configure.ac|2 +-
 src/modules/module-tunnel.c |   91 +--
 src/pulse/def.h |   15 ++
 src/pulse/introspect.c  |   16 +++
 src/pulse/introspect.h  |2 +
 src/pulsecore/protocol-native.c |4 ++
 src/pulsecore/sink.h|1 +
 8 files changed, 88 insertions(+), 53 deletions(-)

diff --git a/PROTOCOL b/PROTOCOL
index 8c69190..8b2f81f 100644
--- a/PROTOCOL
+++ b/PROTOCOL
@@ -278,6 +278,16 @@ New field in PA_COMMAND_UNDERFLOW:
 
 int64_t index
 
+## v24, implemented by = 2.0
+
+New field in all commands that send/receive port introspection data
+(PA_COMMAND_GET_(SOURCE|SINK)_OUTPUT_INFO,
+PA_COMMAND_GET_(SOURCE|SINK)_OUTPUT_INFO_LIST):
+
+uint32_t available
+
+The field is added once for every port.
+
  If you just changed the protocol, read this
 ## module-tunnel depends on the sink/source/sink-input/source-input protocol
 ## internals, so if you changed these, you might have broken module-tunnel.
diff --git a/configure.ac b/configure.ac
index 0bf40a8..d57dbc5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,7 +36,7 @@ AC_SUBST(PA_MINOR, pa_minor)
 AC_SUBST(PA_MAJORMINOR, pa_major.pa_minor)
 
 AC_SUBST(PA_API_VERSION, 12)
-AC_SUBST(PA_PROTOCOL_VERSION, 23)
+AC_SUBST(PA_PROTOCOL_VERSION, 24)
 
 # The stable ABI for client applications, for the version info x:y:z
 # always will hold y=z
diff --git a/src/modules/module-tunnel.c b/src/modules/module-tunnel.c
index faee995..0422c87 100644
--- a/src/modules/module-tunnel.c
+++ b/src/modules/module-tunnel.c
@@ -996,6 +996,41 @@ fail:
 pa_module_unload_request(u-module, TRUE);
 }
 
+static pa_bool_t read_ports(struct userdata *u, pa_tagstruct *t)
+{
+if (u-version = 16) {
+uint32_t n_ports;
+const char *s;
+
+if (pa_tagstruct_getu32(t, n_ports)) {
+pa_log(Parse failure);
+return FALSE;
+}
+
+for (uint32_t j = 0; j  n_ports; j++) {
+uint32_t priority;
+
+if (pa_tagstruct_gets(t, s)  0 || /* name */
+pa_tagstruct_gets(t, s)  0 || /* description */
+pa_tagstruct_getu32(t, priority)  0) {
+
+pa_log(Parse failure);
+return FALSE;
+}
+if (u-version = 24  pa_tagstruct_getu32(t, priority)  0) { 
/* available */
+pa_log(Parse failure);
+return FALSE;
+}
+}
+
+if (pa_tagstruct_gets(t, s)  0) { /* active port */
+pa_log(Parse failure);
+return FALSE;
+}
+}
+return TRUE;
+}
+
 #ifdef TUNNEL_SINK
 
 /* Called from main context */
@@ -1066,32 +1101,8 @@ static void sink_info_cb(pa_pdispatch *pd, uint32_t 
command,  uint32_t tag, pa_t
 }
 }
 
-if (u-version = 16) {
-uint32_t n_ports;
-const char *s;
-
-if (pa_tagstruct_getu32(t, n_ports)) {
-pa_log(Parse failure);
-goto fail;
-}
-
-for (uint32_t j = 0; j  n_ports; j++) {
-uint32_t priority;
-
-if (pa_tagstruct_gets(t, s)  0 || /* name */
-pa_tagstruct_gets(t, s)  0 || /* description */
-pa_tagstruct_getu32(t, priority)  0) {
-
-pa_log(Parse failure);
-goto fail;
-}
-}
-
-if (pa_tagstruct_gets(t, s)  0) { /* active port */
-pa_log(Parse failure);
-goto fail;
-}
-}
+if (!read_ports(u, t))
+goto fail;
 
 if (u-version = 21) {
 uint8_t n_formats;
@@ -1318,32 +1329,8 @@ static void source_info_cb(pa_pdispatch *pd, uint32_t 
command,  uint32_t tag, pa
 }
 }
 
-if (u-version = 16) {
-uint32_t n_ports;
-const char *s;
-
-if (pa_tagstruct_getu32(t, n_ports)) {
-pa_log(Parse failure);
-goto fail;
-}
-
-for (uint32_t j = 0; j  n_ports; j++) {
-uint32_t priority;
-
-if (pa_tagstruct_gets(t, s)  0 || /* name */
-pa_tagstruct_gets(t, s)  0 || /* description */
-pa_tagstruct_getu32(t, priority)  0) {
-
-pa_log(Parse failure);
-goto fail;
-}
-}
-
-if (pa_tagstruct_gets(t, s)  0) { /* active port */
-pa_log(Parse failure);
-goto fail;
-}
-}
+if (!read_ports(u, t))
+goto fail;
 
 if (!pa_tagstruct_eof(t)) {
 pa_log(Packet too long);
diff --git a/src/pulse/def.h b/src/pulse/def.h
index f43e864..8a74fad 100644
--- a/src/pulse/def.h
+++ b/src/pulse/def.h
@@ -967,6 +967,21 @@ typedef void (*pa_free_cb_t)(void *p);
  * playback, \since 1.0 */
 #define

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-18 Thread Pierre-Louis Bossart 
 Subject: [pulseaudio-discuss] [PATCH] Introduce available concept for
 ports, and communicate that to clients. Bump protocol version to 24.
 ---
  PROTOCOL|   10 
  configure.ac|2 +-
  src/modules/module-tunnel.c |   91 +--
 
  src/pulse/def.h |   15 ++
  src/pulse/introspect.c  |   16 +++
  src/pulse/introspect.h  |2 +
  src/pulsecore/protocol-native.c |4 ++
  src/pulsecore/sink.h|1 +
  8 files changed, 88 insertions(+), 53 deletions(-)

Bear with my late feedback, but how exactly is this used by clients? I only
see the context information being changed? Or did you mean this information
is provided mainly to configuration clients/user interfaces for port
selection?
Thanks
-Pierre

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-18 Thread David Henningsson 

On 10/18/2011 10:44 PM, Pierre-Louis Bossart wrote:

Subject: [pulseaudio-discuss] [PATCH] Introduce available concept for
ports, and communicate that to clients. Bump protocol version to 24.
---
  PROTOCOL|   10 
  configure.ac|2 +-
  src/modules/module-tunnel.c |   91 +--

  src/pulse/def.h |   15 ++
  src/pulse/introspect.c  |   16 +++
  src/pulse/introspect.h  |2 +
  src/pulsecore/protocol-native.c |4 ++
  src/pulsecore/sink.h|1 +
  8 files changed, 88 insertions(+), 53 deletions(-)


Bear with my late feedback, but how exactly is this used by clients?I only
see the context information being changed? Or did you mean this information
is provided mainly to configuration clients/user interfaces for port
selection?


Did you miss my previous explanation, or did you find it insufficient? 
I'm repeating it below:


The protocol skew in Ubuntu 11.10 was actually a mistake on my part. 
Since the UI changes that would depend on this information being 
available was backed out, I probably should have backed the actual 
protocol change out as well.


Anyway, here is the patch that forms one of the base features for jack 
detection, and brings upstream out of protocol skew with Ubuntu 11.10.


--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-18 Thread Pierre-Louis Bossart 
 Did you miss my previous explanation, or did you find it insufficient?
 I'm repeating it below:
 
 The protocol skew in Ubuntu 11.10 was actually a mistake on my part.
 Since the UI changes that would depend on this information being
 available was backed out, I probably should have backed the actual
 protocol change out as well.
 
 Anyway, here is the patch that forms one of the base features for jack
 detection, and brings upstream out of protocol skew with Ubuntu 11.10.

I honestly did not understand the explanation, most likely because I use
Fedora for historical reasons and have no idea about the content of Ubuntu
11.10...What type of client will make use of this information?
-Pierre
 

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-18 Thread David Henningsson 

On 10/19/2011 12:11 AM, Pierre-Louis Bossart wrote:

Did you miss my previous explanation, or did you find it insufficient?
I'm repeating it below:

The protocol skew in Ubuntu 11.10 was actually a mistake on my part.
Since the UI changes that would depend on this information being
available was backed out, I probably should have backed the actual
protocol change out as well.

Anyway, here is the patch that forms one of the base features for jack
detection, and brings upstream out of protocol skew with Ubuntu 11.10.


I honestly did not understand the explanation, most likely because I use
Fedora for historical reasons and have no idea about the content of Ubuntu
11.10...What type of client will make use of this information?


My use case is volume control UIs (pavucontrol, gnome volume control, 
etc) - they can use the information to hide three of the four HDMI 
ports, keeping only the right one. Or hide headphones when they are not 
plugged in.


But in Ubuntu 11.10 no client actually uses this information, which is 
why the protocol skew was a mistake.


--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [PATCH] Introduce available concept for ports, and communicate that to clients. Bump protocol version to 24.

2011-10-12 Thread David Henningsson 
Note: There is still no notification when status availability changes.

Signed-off-by: David Henningsson david.hennings...@canonical.com
---

The protocol skew in Ubuntu 11.10 was actually a mistake on my part. Since the 
UI
changes that would depend on this information being available was backed out,
I probably should have backed the actual protocol change out as well. 

Anyway, here is the patch that forms one of the base features for jack 
detection,
and brings upstream out of protocol skew with Ubuntu 11.10.

(Reposted: Thanks courmisch for finding a typo.)

 PROTOCOL|7 +++
 configure.ac|2 +-
 src/modules/module-tunnel.c |   91 +--
 src/pulse/def.h |   15 ++
 src/pulse/introspect.c  |   16 +++
 src/pulse/introspect.h  |2 +
 src/pulsecore/protocol-native.c |4 ++
 src/pulsecore/sink.h|1 +
 8 files changed, 85 insertions(+), 53 deletions(-)

diff --git a/PROTOCOL b/PROTOCOL
index 8c69190..b8b61e4 100644
--- a/PROTOCOL
+++ b/PROTOCOL
@@ -278,6 +278,13 @@ New field in PA_COMMAND_UNDERFLOW:
 
 int64_t index
 
+## v24, implemented by = 2.0
+
+New field in all commands that send/receive port introspection data
+(PA_COMMAND_GET_(SOURCE|SINK)_INFO, PA_COMMAND_GET_(SOURCE|SINK)_INFO_LIST]):
+
+uint32_t available
+
  If you just changed the protocol, read this
 ## module-tunnel depends on the sink/source/sink-input/source-input protocol
 ## internals, so if you changed these, you might have broken module-tunnel.
diff --git a/configure.ac b/configure.ac
index 0bf40a8..d57dbc5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,7 +36,7 @@ AC_SUBST(PA_MINOR, pa_minor)
 AC_SUBST(PA_MAJORMINOR, pa_major.pa_minor)
 
 AC_SUBST(PA_API_VERSION, 12)
-AC_SUBST(PA_PROTOCOL_VERSION, 23)
+AC_SUBST(PA_PROTOCOL_VERSION, 24)
 
 # The stable ABI for client applications, for the version info x:y:z
 # always will hold y=z
diff --git a/src/modules/module-tunnel.c b/src/modules/module-tunnel.c
index faee995..0422c87 100644
--- a/src/modules/module-tunnel.c
+++ b/src/modules/module-tunnel.c
@@ -996,6 +996,41 @@ fail:
 pa_module_unload_request(u-module, TRUE);
 }
 
+static pa_bool_t read_ports(struct userdata *u, pa_tagstruct *t)
+{
+if (u-version = 16) {
+uint32_t n_ports;
+const char *s;
+
+if (pa_tagstruct_getu32(t, n_ports)) {
+pa_log(Parse failure);
+return FALSE;
+}
+
+for (uint32_t j = 0; j  n_ports; j++) {
+uint32_t priority;
+
+if (pa_tagstruct_gets(t, s)  0 || /* name */
+pa_tagstruct_gets(t, s)  0 || /* description */
+pa_tagstruct_getu32(t, priority)  0) {
+
+pa_log(Parse failure);
+return FALSE;
+}
+if (u-version = 24  pa_tagstruct_getu32(t, priority)  0) { 
/* available */
+pa_log(Parse failure);
+return FALSE;
+}
+}
+
+if (pa_tagstruct_gets(t, s)  0) { /* active port */
+pa_log(Parse failure);
+return FALSE;
+}
+}
+return TRUE;
+}
+
 #ifdef TUNNEL_SINK
 
 /* Called from main context */
@@ -1066,32 +1101,8 @@ static void sink_info_cb(pa_pdispatch *pd, uint32_t 
command,  uint32_t tag, pa_t
 }
 }
 
-if (u-version = 16) {
-uint32_t n_ports;
-const char *s;
-
-if (pa_tagstruct_getu32(t, n_ports)) {
-pa_log(Parse failure);
-goto fail;
-}
-
-for (uint32_t j = 0; j  n_ports; j++) {
-uint32_t priority;
-
-if (pa_tagstruct_gets(t, s)  0 || /* name */
-pa_tagstruct_gets(t, s)  0 || /* description */
-pa_tagstruct_getu32(t, priority)  0) {
-
-pa_log(Parse failure);
-goto fail;
-}
-}
-
-if (pa_tagstruct_gets(t, s)  0) { /* active port */
-pa_log(Parse failure);
-goto fail;
-}
-}
+if (!read_ports(u, t))
+goto fail;
 
 if (u-version = 21) {
 uint8_t n_formats;
@@ -1318,32 +1329,8 @@ static void source_info_cb(pa_pdispatch *pd, uint32_t 
command,  uint32_t tag, pa
 }
 }
 
-if (u-version = 16) {
-uint32_t n_ports;
-const char *s;
-
-if (pa_tagstruct_getu32(t, n_ports)) {
-pa_log(Parse failure);
-goto fail;
-}
-
-for (uint32_t j = 0; j  n_ports; j++) {
-uint32_t priority;
-
-if (pa_tagstruct_gets(t, s)  0 || /* name */
-pa_tagstruct_gets(t, s)  0 || /* description */
-pa_tagstruct_getu32(t, priority)  0) {
-
-pa_log(Parse failure);
-goto fail;
-}
-}
-
-if (pa_tagstruct_gets(t, s)  0) { /* active port */
-pa_log(Parse failure);
-