[pulseaudio-discuss] [PATCH 1/4] stream-restore: add volume_is_absolute bool in client side

[Lennart Poettering]

On Fri, 08.05.09 00:45, Marc-André Lureau (marcandre.lureau at gmail.com) wrote:

> From: Marc-André Lureau <marc-andre.lureau at nokia.com>
> 
> While trying to keep ABI compatibility, I introduce a bitfield for the
> boolean volume_is_absolute. We can't extend the struct because the
> API is using pa_ext_stream_restore_info data[] directly..

Looks mostly good.

> @@ -24,6 +24,7 @@
>  
>  #include <pulse/context.h>
>  #include <pulse/version.h>
> +#include <pulsecore/macro.h>

No! pulsecore is private API. pulse/ext-stream-restore.h is public
API. We cannot include this here!

>  
>  /** \file
>   *
> @@ -39,7 +40,8 @@ typedef struct pa_ext_stream_restore_info {
>      pa_channel_map channel_map;  /**< The channel map for the volume field, if applicable */
>      pa_cvolume volume;           /**< The volume of the stream when it was seen last, if applicable and saved */
>      const char *device;          /**< The sink/source of the stream when it was last seen, if applicable and saved */
> -    int mute;                    /**< The boolean mute state of the stream when it was last seen, if applicable and saved */
> +    int mute:1;                  /**< The boolean mute state of the stream when it was last seen, if applicable and saved */
> +    pa_bool_t volume_is_absolute:1; /**< True if the volume is absolute, if applicable and saved.  \since 0.9.16 */
>  } pa_ext_stream_restore_info;

pa_bool_t is used internally only. In the public API we only expose
ints. pa_bool_t is defined as _Bool on C99 and int on other
compilers. This discrepancy should not be visible to outside due to
ABI stability issues.

Please don't use :1 for this. This breaks ABI.

A dirty trick would be to encode this flag in device[strlen(device)+1]
or so and hide that in a macro. But that would break if device is
NULL.

Hmm, this situation really sucks I must admit.

I am tempted to suggest introducing ext-stream-restore2.h which
includes a much cleaned up and more extensible interface and copies
the old interface mostly 1:1 except where it doesn't make sense.

With that we could also enforce that TEST opcode.

Alternatively just ad a new struct pa_ext_stream_restore_info2 which
is fixed and two new calls to read and write those fields. Instead of
passing arrays of this stuff around we should pass arrays of pointers
around.

Seems Lennart didn't use his brains when he designed this.

If we need to redesign this API then let's make it flexible enough to
cater for the features Colin and I discussed on this list a while
back.

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/           GnuPG 0x1A015CC4