[pulseaudio-discuss] [PATCH] Introduce "available" concept for ports, and communicate that to clients. Bump protocol version to 24.
Tanu Kaskinen
tanuk at iki.fi
Tue Oct 18 10:16:20 PDT 2011
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.henningsson at 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 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 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
> +
> +/** \endcond */
> +
> PA_C_DECL_END
>
> #endif
> diff --git a/src/pulse/introspect.c b/src/pulse/introspect.c
> index 0d1d03f..c8bf7ca 100644
> --- a/src/pulse/introspect.c
> +++ b/src/pulse/introspect.c
> @@ -211,6 +211,14 @@ static void context_get_sink_info_callback(pa_pdispatch *pd, uint32_t command, u
> goto fail;
> }
>
> + i.ports[0][j].available = PA_PORT_AVAILABLE_UNKNOWN;
> + if (o->context->version >= 24) {
> + uint32_t av;
> + if (pa_tagstruct_getu32(t, &av) < 0 || av > PA_PORT_AVAILABLE_YES)
> + goto fail;
> + i.ports[0][j].available = av;
> + }
> +
> i.ports[j] = &i.ports[0][j];
> }
>
> @@ -476,6 +484,14 @@ static void context_get_source_info_callback(pa_pdispatch *pd, uint32_t command,
> goto fail;
> }
>
> + i.ports[0][j].available = PA_PORT_AVAILABLE_UNKNOWN;
> + if (o->context->version >= 24) {
> + uint32_t av;
> + if (pa_tagstruct_getu32(t, &av) < 0 || av > PA_PORT_AVAILABLE_YES)
> + goto fail;
> + i.ports[0][j].available = av;
> + }
> +
> i.ports[j] = &i.ports[0][j];
> }
>
> 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;
>
> /** Stores information about sources. Please note that this structure
> diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
> index 0ee4ead..646ecbf 100644
> --- a/src/pulsecore/protocol-native.c
> +++ b/src/pulsecore/protocol-native.c
> @@ -3102,6 +3102,8 @@ static void sink_fill_tagstruct(pa_native_connection *c, pa_tagstruct *t, pa_sin
> pa_tagstruct_puts(t, p->name);
> pa_tagstruct_puts(t, p->description);
> pa_tagstruct_putu32(t, p->priority);
> + if (c->version >= 24)
> + pa_tagstruct_putu32(t, p->available);
> }
> }
>
> @@ -3173,6 +3175,8 @@ static void source_fill_tagstruct(pa_native_connection *c, pa_tagstruct *t, pa_s
> pa_tagstruct_puts(t, p->name);
> pa_tagstruct_puts(t, p->description);
> pa_tagstruct_putu32(t, p->priority);
> + if (c->version >= 24)
> + pa_tagstruct_putu32(t, p->available);
> }
> }
>
> 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 */
This is a server-side header - doxygen syntax shouldn't be used, and the
\since tag is not needed. I'm not sure the whole comment is very useful,
but it does no harm either.
> /* .. followed by some implementation specific data */
> };
--
Tanu
More information about the pulseaudio-discuss
mailing list