Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
'Twas brillig, and Tanu Kaskinen at 18/04/11 10:31 did gyre and gimble: For publishing APIs: /* Returns a negative error code if the extra api is already registered. */ int pa_extra_api_register(const char *name, void *api); void pa_extra_api_unregister(const_char *name); For consuming APIs: /* Returns the currently registered api object, * or NULL if the api isn't registered right now. */ void *pa_extra_api_get(const char *name); Additionally, there would be a couple of hooks defined for getting notifications about APIs getting registered and unregistered. That seems sensible. The api object would be a struct with function pointers. The API provider would create instances of that struct and provide the function implementation. The API consumer would talk to the provider using those function pointers. OK, that all seems fine IMO. The api object header could be located in src/extra_apis - that would make the api not directly dependent on a particular module. If the API is not dependant on a particular module, what code calls pa_extra_api_register? I'd be happy enough defining this extra API as an IMC (c.f. IPC) system and thus make it very much tied to modules. You could even have a call tracker module that just provides the code you posted earlier with nothing more than a .h file and pa__init and pa__done implementations and make sure this is loaded before any modules that happen to want to use it. I've been using the term extra api here. I don't think it's the greatest name in the world, but that's the best I could think of. I'd like extension more, but that word is already used for other purposes. Yeah extension is already in use for protocol extensions I guess. How about pa_imc_*? (for inter-module comms) or is that perhaps too cryptic? 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@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
On Wed, 2011-04-20 at 11:36 +0300, Colin Guthrie wrote: The api object header could be located in src/extra_apis - that would make the api not directly dependent on a particular module. If the API is not dependant on a particular module, what code calls pa_extra_api_register? I'd be happy enough defining this extra API as an IMC (c.f. IPC) system and thus make it very much tied to modules. Hmm... did you get the impression that these apis would not necessarily be tied to any modules at all? That was not the idea - the apis would be implemented always by modules, but not necessarily by any particular module. For example, if there was an api for getting the current call state, it could be implemented by multiple modules, each of which would use different logic for determining whether a call is active or not. The modules would of course conflict with each other, so they could not be loaded at the same time. This would be handled by pa_extra_api_register() - only one module could have the api registered at any given time, the other modules would get an error when they try to call pa_extra_api_register(). I've been using the term extra api here. I don't think it's the greatest name in the world, but that's the best I could think of. I'd like extension more, but that word is already used for other purposes. Yeah extension is already in use for protocol extensions I guess. How about pa_imc_*? (for inter-module comms) or is that perhaps too cryptic? Yeah, it's cryptic, but it's also shorter. And extra api isn't an obvious name either. I'm not sure which I like more. Probably imc. Regarding function names, they will actually have form pa_imc_manager_* or pa_extra_api_manager_* if I get to decide... The functions can be methods of either some manager object or pa_core. If they're implemented directly by pa_core, then the prefix will of course be pa_core_, but I'd prefer having a separate manager object. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
'Twas brillig, and Tanu Kaskinen at 20/04/11 11:54 did gyre and gimble: On Wed, 2011-04-20 at 11:36 +0300, Colin Guthrie wrote: The api object header could be located in src/extra_apis - that would make the api not directly dependent on a particular module. If the API is not dependant on a particular module, what code calls pa_extra_api_register? I'd be happy enough defining this extra API as an IMC (c.f. IPC) system and thus make it very much tied to modules. Hmm... did you get the impression that these apis would not necessarily be tied to any modules at all? That was not the idea - the apis would be implemented always by modules, but not necessarily by any particular module. For example, if there was an api for getting the current call state, it could be implemented by multiple modules, each of which would use different logic for determining whether a call is active or not. The modules would of course conflict with each other, so they could not be loaded at the same time. This would be handled by pa_extra_api_register() - only one module could have the api registered at any given time, the other modules would get an error when they try to call pa_extra_api_register(). Ahh I get what you mean :) Yeah this all makes sense. Thanks for clarifying :) I've been using the term extra api here. I don't think it's the greatest name in the world, but that's the best I could think of. I'd like extension more, but that word is already used for other purposes. Yeah extension is already in use for protocol extensions I guess. How about pa_imc_*? (for inter-module comms) or is that perhaps too cryptic? Yeah, it's cryptic, but it's also shorter. And extra api isn't an obvious name either. I'm not sure which I like more. Probably imc. Regarding function names, they will actually have form pa_imc_manager_* or pa_extra_api_manager_* if I get to decide... The functions can be methods of either some manager object or pa_core. If they're implemented directly by pa_core, then the prefix will of course be pa_core_, but I'd prefer having a separate manager object. OK, so the manager object is separate but kept in the core? Seems fine by me to keep things neatly separated. I thinking of taking a pop at this at some point soon (if you don't beat me to it), so can we decide on the name now? Anyone got any better naming suggestions? pa_imc_manager_* (Pro: short, Con: Inter Module Comms cryptic) pa_extra_api_manager_* (Pro: clear, Con: long, Extension vs. Extra which is which and for what purpose?) pa_xapi_manager_* (Pro: short, Con: might be confused with X11) pa_mxapi_manager_* (Pro: quite short, Con: Module eXtension API but still quite cryptic) I'm not really blown away by any of them :s 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@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
On Wed, 2011-04-20 at 15:19 +0300, Colin Guthrie wrote: OK, so the manager object is separate but kept in the core? Seems fine by me to keep things neatly separated. Yep. I thinking of taking a pop at this at some point soon (if you don't beat me to it), so can we decide on the name now? Anyone got any better naming suggestions? pa_imc_manager_* (Pro: short, Con: Inter Module Comms cryptic) pa_extra_api_manager_* (Pro: clear, Con: long, Extension vs. Extra which is which and for what purpose?) pa_xapi_manager_* (Pro: short, Con: might be confused with X11) pa_mxapi_manager_* (Pro: quite short, Con: Module eXtension API but still quite cryptic) I'm not really blown away by any of them :s I'll vote for pa_imc_manager_*. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
'Twas brillig, and Tanu Kaskinen at 06/04/11 16:23 did gyre and gimble: On Wed, 2011-04-06 at 14:23 +0100, Colin Guthrie wrote: 'Twas brillig, and Tanu Kaskinen at 06/04/11 12:33 did gyre and gimble: +/* This is a shared singleton object, currently used by Meego's voice and + * policy enforcement modules. The purpose of the object is just to maintain + * a boolean state of call is active or call is not active, and to provide + * notification hooks for tracking state changes. So one module will be setting + * the state (the voice module) and one or more modules will follow the state + * through the hooks (the policy enforcement module). */ Could I use this approach to, for example, extend module-stream-restore and module-device-restore, to save particular keys in a stream, or device proplist? snip I don't see any reason why you couldn't. Just realised I quoted totally the wrong bit there I meant to quote: I'd be interested in implementing at some point (no promises or timelines) a small framework for making inter-module communication easier, or at least cleaner (this kind of hacks in pulsecore are actually very easy to work with, but clean they are not). The main motivation for this would be ripping out the dbus stuff from module-stream-restore. The dbus API implementation in module-stream-restore.c takes about a half of the whole file, which makes reading the stream-restore code more difficult than it should be. I'd like to keep the dbus interface implementation in module-dbus-protocol only. For this to be possible, there would need to be some way for the modules to talk to each other. It could be solved in a similar way as this call-state-tracker is done, but I'd prefer a generic framework that modules could use to publish extra APIs that other modules can then use. Which probably makes my comments seem a little more sane :D Do you have any specific requirements for this inter-module IPC? Do we just need a hook system with random data pointer + userdata? The random data pointer would be specific to the caller/callee pair and the userdata would be as per usual. Anything more specific than that needed? 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@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
On Mon, 2011-04-18 at 11:35 +0300, Colin Guthrie wrote: I'd be interested in implementing at some point (no promises or timelines) a small framework for making inter-module communication easier, or at least cleaner (this kind of hacks in pulsecore are actually very easy to work with, but clean they are not). The main motivation for this would be ripping out the dbus stuff from module-stream-restore. The dbus API implementation in module-stream-restore.c takes about a half of the whole file, which makes reading the stream-restore code more difficult than it should be. I'd like to keep the dbus interface implementation in module-dbus-protocol only. For this to be possible, there would need to be some way for the modules to talk to each other. It could be solved in a similar way as this call-state-tracker is done, but I'd prefer a generic framework that modules could use to publish extra APIs that other modules can then use. Which probably makes my comments seem a little more sane :D Do you have any specific requirements for this inter-module IPC? Do we just need a hook system with random data pointer + userdata? The random data pointer would be specific to the caller/callee pair and the userdata would be as per usual. Anything more specific than that needed? I'm not sure what you exactly mean. This is what I had in mind for the stuff that needs to be added to the core, I guess it's pretty close to what you described: For publishing APIs: /* Returns a negative error code if the extra api is already registered. */ int pa_extra_api_register(const char *name, void *api); void pa_extra_api_unregister(const_char *name); For consuming APIs: /* Returns the currently registered api object, * or NULL if the api isn't registered right now. */ void *pa_extra_api_get(const char *name); Additionally, there would be a couple of hooks defined for getting notifications about APIs getting registered and unregistered. The api object would be a struct with function pointers. The API provider would create instances of that struct and provide the function implementation. The API consumer would talk to the provider using those function pointers. The api object header could be located in src/extra_apis - that would make the api not directly dependent on a particular module. I don't really have real-world requirements for generic apis that could be provided by multiple modules, but I can't see any costs either for this genericity, other than some minimal added clutter in the src directory. I've been using the term extra api here. I don't think it's the greatest name in the world, but that's the best I could think of. I'd like extension more, but that word is already used for other purposes. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
From: Tanu Kaskinen ext-tanu.kaski...@nokia.com
See call-state-tracker.h for details.
Changed in patch v2:
Don't take a reference of pa_core. The previous version would have
reintroduced the bug that was fixed in 1e381fbf.
---
src/Makefile.am|1 +
src/pulsecore/call-state-tracker.c | 125
src/pulsecore/call-state-tracker.h | 54 +++
3 files changed, 180 insertions(+), 0 deletions(-)
create mode 100644 src/pulsecore/call-state-tracker.c
create mode 100644 src/pulsecore/call-state-tracker.h
diff --git a/src/Makefile.am b/src/Makefile.am
index bdedded..85c5602 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -835,6 +835,7 @@ libpulsecore_@PA_MAJORMINOR@_la_SOURCES = \
pulsecore/asyncmsgq.c pulsecore/asyncmsgq.h \
pulsecore/asyncq.c pulsecore/asyncq.h \
pulsecore/auth-cookie.c pulsecore/auth-cookie.h \
+ pulsecore/call-state-tracker.c pulsecore/call-state-tracker.h \
pulsecore/cli-command.c pulsecore/cli-command.h \
pulsecore/cli-text.c pulsecore/cli-text.h \
pulsecore/client.c pulsecore/client.h \
diff --git a/src/pulsecore/call-state-tracker.c
b/src/pulsecore/call-state-tracker.c
new file mode 100644
index 000..0ac9be5
--- /dev/null
+++ b/src/pulsecore/call-state-tracker.c
@@ -0,0 +1,125 @@
+/***
+ This file is part of PulseAudio.
+
+ Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+
+ PulseAudio is free software; you can redistribute it and/or modify
+ it under the terms of the GNU Lesser General Public License as published
+ by the Free Software Foundation; either version 2.1 of the License,
+ or (at your option) any later version.
+
+ PulseAudio is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with PulseAudio; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ USA.
+***/
+
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
+#include pulsecore/core.h
+#include pulsecore/hook-list.h
+#include pulsecore/log.h
+#include pulsecore/macro.h
+#include pulsecore/refcnt.h
+#include pulsecore/shared.h
+
+#include call-state-tracker.h
+
+struct pa_call_state_tracker {
+PA_REFCNT_DECLARE;
+
+pa_core *core;
+pa_bool_t active;
+pa_hook hooks[PA_CALL_STATE_HOOK_MAX];
+};
+
+static pa_call_state_tracker* call_state_tracker_new(pa_core *c) {
+pa_call_state_tracker *t;
+pa_call_state_hook_t h;
+
+pa_assert(c);
+
+t = pa_xnew0(pa_call_state_tracker, 1);
+PA_REFCNT_INIT(t);
+t-core = c;
+t-active = FALSE;
+
+for (h = 0; h PA_CALL_STATE_HOOK_MAX; h++)
+pa_hook_init(t-hooks[h], t);
+
+pa_assert_se(pa_shared_set(c, call-state-tracker, t) = 0);
+
+return t;
+}
+
+pa_call_state_tracker *pa_call_state_tracker_get(pa_core *core) {
+pa_call_state_tracker *t;
+
+if ((t = pa_shared_get(core, call-state-tracker)))
+return pa_call_state_tracker_ref(t);
+
+return call_state_tracker_new(core);
+}
+
+pa_call_state_tracker *pa_call_state_tracker_ref(pa_call_state_tracker *t) {
+pa_assert(t);
+pa_assert(PA_REFCNT_VALUE(t) = 1);
+
+PA_REFCNT_INC(t);
+
+return t;
+}
+
+void pa_call_state_tracker_unref(pa_call_state_tracker *t) {
+pa_call_state_hook_t h;
+
+pa_assert(t);
+pa_assert(PA_REFCNT_VALUE(t) = 1);
+
+if (PA_REFCNT_DEC(t) 0)
+return;
+
+for (h = 0; h PA_CALL_STATE_HOOK_MAX; h++)
+pa_hook_done(t-hooks[h]);
+
+pa_assert_se(pa_shared_remove(t-core, call-state-tracker) = 0);
+
+pa_xfree(t);
+}
+
+pa_bool_t pa_call_state_tracker_get_active(pa_call_state_tracker *t) {
+pa_assert(t);
+pa_assert(PA_REFCNT_VALUE(t) = 1);
+
+return t-active;
+}
+
+void pa_call_state_tracker_set_active(pa_call_state_tracker *t, pa_bool_t
active) {
+pa_bool_t changed;
+
+pa_assert(t);
+pa_assert(PA_REFCNT_VALUE(t) = 1);
+
+changed = active != t-active;
+
+t-active = active;
+
+if (changed)
+pa_hook_fire(t-hooks[PA_CALL_STATE_HOOK_CHANGED], (void *) active);
+
+pa_log_debug(Call state set %s (%s), active ? active : inactive,
changed ? changed : not changed);
+}
+
+pa_hook *pa_call_state_tracker_hooks(pa_call_state_tracker *t) {
+pa_assert(t);
+pa_assert(PA_REFCNT_VALUE(t) = 1);
+
+return t-hooks;
+}
diff --git a/src/pulsecore/call-state-tracker.h
b/src/pulsecore/call-state-tracker.h
new file mode 100644
index 000..9a6c60b
--- /dev/null
+++ b/src/pulsecore/call-state-tracker.h
@@ -0,0 +1,54 @@
+#ifndef foocallstatetrackerhfoo
+#define foocallstatetrackerhfoo
+
+/***
+ This file is part of PulseAudio.
+
+ Copyright
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
'Twas brillig, and Tanu Kaskinen at 06/04/11 12:33 did gyre and gimble: +/* This is a shared singleton object, currently used by Meego's voice and + * policy enforcement modules. The purpose of the object is just to maintain + * a boolean state of call is active or call is not active, and to provide + * notification hooks for tracking state changes. So one module will be setting + * the state (the voice module) and one or more modules will follow the state + * through the hooks (the policy enforcement module). */ Could I use this approach to, for example, extend module-stream-restore and module-device-restore, to save particular keys in a stream, or device proplist? e.g. I could request from (a yet to be written) module-filter-apply module that we save filter.want properties on streams? Another thing I would use it for would be to save whether the volume channel was split or not in the GUIs (e.g. the Lock Channels button in pavucontrol) so that this is preserved across boots/stream departure/arrival. I've probably not explained that well. 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@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
On Wed, 2011-04-06 at 14:23 +0100, Colin Guthrie wrote: 'Twas brillig, and Tanu Kaskinen at 06/04/11 12:33 did gyre and gimble: +/* This is a shared singleton object, currently used by Meego's voice and + * policy enforcement modules. The purpose of the object is just to maintain + * a boolean state of call is active or call is not active, and to provide + * notification hooks for tracking state changes. So one module will be setting + * the state (the voice module) and one or more modules will follow the state + * through the hooks (the policy enforcement module). */ Could I use this approach to, for example, extend module-stream-restore and module-device-restore, to save particular keys in a stream, or device proplist? snip I don't see any reason why you couldn't. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
