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

[Marc-André Lureau]

Hi

On Sat, May 9, 2009 at 1:39 AM, Lennart Poettering
<lennart at poettering.net> wrote:
> 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..
>
>
>> +#include <pulsecore/macro.h>
>
> No! pulsecore is private API. pulse/ext-stream-restore.h is public
> API. We cannot include this here!
>
>
> 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.

That is one of the reason why I wanted to resend. (I didn't see in
first place because I have private headers installed, a bit like the
Intel guys patch :)

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

Unfortunately, it does :(

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

I see, you don't even consider using some of the bits of "int mute".
That is understandable.

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

Looks like there is no other choice...

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

Ok, I will look back in archives. Do you have already recommendations
to make, like use only array of pointers of structs for a start?

thanks!

-- 
Marc-André Lureau