Re: [pulseaudio-discuss] [PATCH] raop: silence a Coverity complaint

[Hajime Fujita]

I thought I added this fix before but apparently not.
Looks good to me. Thank you for bringing this up.


Hajime

> On Jul 27, 2017, at 8:07 PM, Tanu Kaskinen  wrote:
> 
> CID: 1398155
> ---
> src/modules/raop/raop-sink.c | 7 +++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/src/modules/raop/raop-sink.c b/src/modules/raop/raop-sink.c
> index e5d219e87..4d13927fc 100644
> --- a/src/modules/raop/raop-sink.c
> +++ b/src/modules/raop/raop-sink.c
> @@ -391,6 +391,13 @@ static void thread_func(void *userdata) {
> if (!pa_raop_client_can_stream(u->raop))
> continue;
> 
> +/* This assertion is meant to silence a complaint from Coverity about
> + * pollfd being possibly NULL when we access it later. That's a false
> + * positive, because we check pa_raop_client_can_stream() above, and 
> if
> + * that returns true, it means that the connection is up, and when 
> the
> + * connection is up, pollfd will be non-NULL. */
> +pa_assert(pollfd);
> +
> if (u->memchunk.length <= 0) {
> if (u->memchunk.memblock)
> pa_memblock_unref(u->memchunk.memblock);
> -- 
> 2.13.2
> 

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

[pulseaudio-discuss] [PATCH] raop: silence a Coverity complaint

[Tanu Kaskinen]

CID: 1398155
---
 src/modules/raop/raop-sink.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/modules/raop/raop-sink.c b/src/modules/raop/raop-sink.c
index e5d219e87..4d13927fc 100644
--- a/src/modules/raop/raop-sink.c
+++ b/src/modules/raop/raop-sink.c
@@ -391,6 +391,13 @@ static void thread_func(void *userdata) {
 if (!pa_raop_client_can_stream(u->raop))
 continue;
 
+/* This assertion is meant to silence a complaint from Coverity about
+ * pollfd being possibly NULL when we access it later. That's a false
+ * positive, because we check pa_raop_client_can_stream() above, and if
+ * that returns true, it means that the connection is up, and when the
+ * connection is up, pollfd will be non-NULL. */
+pa_assert(pollfd);
+
 if (u->memchunk.length <= 0) {
 if (u->memchunk.memblock)
 pa_memblock_unref(u->memchunk.memblock);
-- 
2.13.2

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

Re: [pulseaudio-discuss] [PATCH 1/4] core: Add generic message interface

[Georg Chini]

On 27.07.2017 07:17, Tanu Kaskinen wrote:

On Fri, 2017-07-21 at 21:25 +0200, Georg Chini wrote:

This patch adds a new feature to the core which allows to exchange
messages between objects. An object can register/unregister a message
handler with pa_core_{register, unregister}_message_handler() while
any other object can check if a handler is registered for some name
with pa_core_find_message_handler() and then call the returned handler.

Splitting the message sending into separate "find handler" and "use
handler" steps seems strange. Why not just pa_core_send_message() (or
maybe "handle" or "post" or "receive" would be better verbs, I'm not
sure)? The pa_core_message_handler struct could then be hidden in
core.c.


I have to think about it, but you are probably right. Although it might
still make sense to have a is_handler_installed() boolean function, so
that the sender can check if handlers for certain objects are present.




The patch is a precondition for the following patches that also allow
clients to send messages to pulseaudio objects.

There is no restriction on object names, but the intention is to use
a path-like syntax, for example /core/sink_1 for a sink or
/name/instances/index for modules. The exact naming convention still
needs to be agreed.

Objects also can have a short name which can be used in several ways.
First objects of the same type should have the same short name, so
that you can either send a message to all instances or to the first
instance of an object.

The current code doesn't seem to provide an option to send a message to
multiple recipients.


Yes, I have not added a "find_all_handlers" function because
I have no current use case, but it could easily be implemented if it
should for example be necessary to send a message to all sinks.
Maybe I should make it more clear that there is no special code
for passing messages to multiple recipients.
If you want to send messages to multiple recipients, you have to iterate
over the objects. Here again, the find_handler function comes in handy,
but it could be solved differently.




pa_core_find_message_handler() returns the
first instance if a short name is specified as search string.
Second, special objects like the default sink/source can have unique
short names to make it easy to send messages to them.

I'd rather not add extra functionality if it's not used. Do you have
plans to add code that uses the short names?


This is not really extra functionality if you want to be able to easily
address the first instance of an object. Consider an object, that has
multiple handlers installed (maybe plugins).
So you might have multiple names that start with /object_prefix. If
you don't have a short name, you need to iterate through the list
of installed handlers to find the best match. With the short name,
you just pick the first handler with a matching short name.
Think of the short name as a category identifier. It seemed too
difficult to find rules that reliably map object name prefixes to object
category, because there is (at least up to now) no rule for  the object
names themselves.



I also think it would be better to not tie the short names directly in
the message handlers. In case there's an alias for the default sink,
then the alias is not a property of the sink message handler, because
the alias can be changed to point to a different sink. I'd think it as
a symlink. The short name/alias/symlink should have its own message
handler, which just forwards the message to the current "target"
object.


I don't understand. There is no alias for the default sink. It's the other
way round, default_sink is an alias for a real object which has a handler.
Which object would install the handler for the default sink? If the alias
is stored with the handler and the default sink changes, you just have
to move the alias name to the handler of the new default sink. That does
not involve setting up any "dummy" handlers.
Using this feature does however not prevent your approach if it is
necessary in some special situation.




---
  src/pulsecore/core-util.c | 71 +++
  src/pulsecore/core-util.h | 22 +++
  src/pulsecore/core.c  |  4 +++
  src/pulsecore/core.h  |  2 +-
  4 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/src/pulsecore/core-util.c b/src/pulsecore/core-util.c
index d4cfa20c..2aa0e2e4 100644
--- a/src/pulsecore/core-util.c
+++ b/src/pulsecore/core-util.c

This code is better suited for core.c, since it modifies the pa_core
state.


OK.




+/* Register message handler. recipient_name must be a unique name. short_name 
can be used
+ * to group objects of the same type, see pa_core_find_message_handler(). 
short_name and
+ * description are optional and may be NULL. */
+int pa_core_register_message_handler(pa_core *c, const char *recipient_name, 
const char *short_name, const char *description, pa_core_message_handler_cb_t 
cb, void *userdata) {

What do you 

Re: [pulseaudio-discuss] [PATCH] bluez5-device: Set transport state correctly for AG role

[Tanu Kaskinen]

On Sat, 2017-07-22 at 10:47 +0200, Georg Chini wrote:
> When connecting a headset via the native backend, the transport state was
> not updated correctly.
> 
> This patch sets the state to PLAYING in transport_acquire() if necessary.
> ---
>  src/modules/bluetooth/module-bluez5-device.c | 20 
>  1 file changed, 20 insertions(+)

Looks good, I pushed this to "next".

-- 
Tanu

https://www.patreon.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss