[pulseaudio-discuss] [PATCH v2 3/4] protocol-native: add "ext_supported" to install_ext()
Lennart Poettering
lennart at poettering.net
Mon May 11 17:23:28 PDT 2009
On Tue, 12.05.09 01:35, Marc-André Lureau (marc-andre.lureau at nokia.com) wrote:
> In order to support different extensions versions in the same module,
> it introduces an array of supported extension strings. They are free
> form (freely matched by extension).
Hmm. I actually thought this a bit differently. i.e. that there would
be no distinction between the extension name and the extension version
string.
Currently extension messages carry a module idx/name which is used to
route the message the appropriate module. My idea was now to
virtualize that and instead of happing the module name 1:1 in that
string simply use a string that is just 'related' to the module name,
but not necessarily identical to the module name. This could then be
used to version interfaces.
i.e. the existing m-s-r is reachable under the extension name
"module-stream-restore". For the updated protocol it would then be
available as "module-stream-restore-2" as well. The client side would
use the nego data to chose to which extension name it should send its
data to. On the server side the received string would be used to parse
the received data correctly.
The behaviour you implemented makes a lot of sense, too.
I need to think a bit more which way we want to have it in the end...
But otherwise this looks godd.
Lennart
> ---
> src/modules/module-stream-restore.c | 3 +-
> src/pulsecore/protocol-native.c | 44 +++++++++++++++++++++++++++++-----
> src/pulsecore/protocol-native.h | 4 ++-
> 3 files changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/src/modules/module-stream-restore.c b/src/modules/module-stream-restore.c
> index 4ff6c57..db30528 100644
> --- a/src/modules/module-stream-restore.c
> +++ b/src/modules/module-stream-restore.c
> @@ -807,6 +807,7 @@ int pa__init(pa_module*m) {
> uint32_t idx;
> pa_bool_t restore_device = TRUE, restore_volume = TRUE, restore_muted = TRUE;
> int gdbm_cache_size;
> + const char* ext_supported[] = { "stream-restore-1", "stream-restore-2", NULL };
>
> pa_assert(m);
>
> @@ -836,7 +837,7 @@ int pa__init(pa_module*m) {
> u->subscribed = pa_idxset_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func);
>
> u->protocol = pa_native_protocol_get(m->core);
> - pa_native_protocol_install_ext(u->protocol, m, extension_cb);
> + pa_native_protocol_install_ext(u->protocol, m, extension_cb, ext_supported);
>
> u->connection_unlink_hook_slot = pa_hook_connect(&pa_native_protocol_hooks(u->protocol)[PA_NATIVE_HOOK_CONNECTION_UNLINK], PA_HOOK_NORMAL, (pa_hook_cb_t) connection_unlink_hook_cb, u);
>
> diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
> index aecaf71..5db080f 100644
> --- a/src/pulsecore/protocol-native.c
> +++ b/src/pulsecore/protocol-native.c
> @@ -159,6 +159,11 @@ PA_DECLARE_CLASS(upload_stream);
> #define UPLOAD_STREAM(o) (upload_stream_cast(o))
> static PA_DEFINE_CHECK_TYPE(upload_stream, output_stream);
>
> +typedef struct native_protocol_ext {
> + pa_native_protocol_ext_cb_t cb;
> + pa_strlist *supported;
> +} native_protocol_ext;
> +
> struct pa_native_connection {
> pa_msgobject parent;
> pa_native_protocol *protocol;
> @@ -4149,7 +4154,7 @@ static void command_extension(pa_pdispatch *pd, uint32_t command, uint32_t tag,
> uint32_t idx = PA_INVALID_INDEX;
> const char *name = NULL;
> pa_module *m;
> - pa_native_protocol_ext_cb_t cb;
> + native_protocol_ext *ext;
>
> pa_native_connection_assert_ref(c);
> pa_assert(t);
> @@ -4177,10 +4182,11 @@ static void command_extension(pa_pdispatch *pd, uint32_t command, uint32_t tag,
> CHECK_VALIDITY(c->pstream, m, tag, PA_ERR_NOEXTENSION);
> CHECK_VALIDITY(c->pstream, m->load_once || idx != PA_INVALID_INDEX, tag, PA_ERR_INVALID);
>
> - cb = (pa_native_protocol_ext_cb_t) (unsigned long) pa_hashmap_get(c->protocol->extensions, m);
> - CHECK_VALIDITY(c->pstream, cb, tag, PA_ERR_NOEXTENSION);
> + ext = (native_protocol_ext *) pa_hashmap_get(c->protocol->extensions, m);
> + CHECK_VALIDITY(c->pstream, ext, tag, PA_ERR_NOEXTENSION);
> + CHECK_VALIDITY(c->pstream, ext->cb, tag, PA_ERR_NOEXTENSION);
>
> - if (cb(c->protocol, m, c, tag, t) < 0)
> + if (ext->cb(c->protocol, m, c, tag, t) < 0)
> protocol_error(c);
> }
>
> @@ -4532,6 +4538,15 @@ pa_native_protocol* pa_native_protocol_ref(pa_native_protocol *p) {
> return p;
> }
>
> +static void extension_free(void *p, void *userdata) {
> + native_protocol_ext *ext;
> +
> + pa_assert_se(ext = p);
> +
> + pa_strlist_free(ext->supported);
> + pa_xfree(ext);
> +}
> +
> void pa_native_protocol_unref(pa_native_protocol *p) {
> pa_native_connection *c;
> pa_native_hook_t h;
> @@ -4552,7 +4567,7 @@ void pa_native_protocol_unref(pa_native_protocol *p) {
> for (h = 0; h < PA_NATIVE_HOOK_MAX; h++)
> pa_hook_done(&p->hooks[h]);
>
> - pa_hashmap_free(p->extensions, NULL, NULL);
> + pa_hashmap_free(p->extensions, extension_free, NULL);
>
> pa_assert_se(pa_shared_remove(p->core, "native-protocol") >= 0);
>
> @@ -4593,23 +4608,38 @@ pa_strlist *pa_native_protocol_servers(pa_native_protocol *p) {
> return p->servers;
> }
>
> -int pa_native_protocol_install_ext(pa_native_protocol *p, pa_module *m, pa_native_protocol_ext_cb_t cb) {
> +int pa_native_protocol_install_ext(pa_native_protocol *p, pa_module *m, pa_native_protocol_ext_cb_t cb, const char *ext_supported[]) {
> + native_protocol_ext *ext;
> + unsigned i;
> +
> pa_assert(p);
> pa_assert(PA_REFCNT_VALUE(p) >= 1);
> pa_assert(m);
> pa_assert(cb);
> + pa_assert(ext_supported);
> + pa_assert(ext_supported[0]);
> pa_assert(!pa_hashmap_get(p->extensions, m));
>
> - pa_assert_se(pa_hashmap_put(p->extensions, m, (void*) (unsigned long) cb) == 0);
> + ext = pa_xnew(native_protocol_ext, 1);
> + ext->cb = cb;
> + ext->supported = NULL;
> + for (i = 0; ext_supported[i] != NULL; ++i)
> + ext->supported = pa_strlist_prepend(ext->supported, ext_supported[i]);
> +
> + pa_assert_se(pa_hashmap_put(p->extensions, m, ext) == 0);
> return 0;
> }
>
> void pa_native_protocol_remove_ext(pa_native_protocol *p, pa_module *m) {
> + native_protocol_ext *ext;
> +
> pa_assert(p);
> pa_assert(PA_REFCNT_VALUE(p) >= 1);
> pa_assert(m);
>
> + pa_assert_se(ext = (native_protocol_ext *) pa_hashmap_get(p->extensions, m));
> pa_assert_se(pa_hashmap_remove(p->extensions, m));
> + extension_free(ext, NULL);
> }
>
> pa_native_options* pa_native_options_new(void) {
> diff --git a/src/pulsecore/protocol-native.h b/src/pulsecore/protocol-native.h
> index 8a8d601..7bd159a 100644
> --- a/src/pulsecore/protocol-native.h
> +++ b/src/pulsecore/protocol-native.h
> @@ -76,7 +76,9 @@ typedef int (*pa_native_protocol_ext_cb_t)(
> uint32_t tag,
> pa_tagstruct *t);
>
> -int pa_native_protocol_install_ext(pa_native_protocol *p, pa_module *m, pa_native_protocol_ext_cb_t cb);
> +/* "extensions_supported" is used to identify extension name and version supported by the module.
> + * ex: ext_supported = { "stream-restore", "stream-restore-2", NULL } */
> +int pa_native_protocol_install_ext(pa_native_protocol *p, pa_module *m, pa_native_protocol_ext_cb_t cb, const char *ext_supported[]);
> void pa_native_protocol_remove_ext(pa_native_protocol *p, pa_module *m);
>
> pa_pstream* pa_native_connection_get_pstream(pa_native_connection *c);
Lennart
--
Lennart Poettering Red Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/ GnuPG 0x1A015CC4
More information about the pulseaudio-discuss
mailing list