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 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?
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
