[pulseaudio-discuss] [PATCH 11/11] pulsecore: srbchannel: Enable memfd support; pump protocol version

[Ahmed S. Darwish]

Hi Tanu,

On Thu, Oct 01, 2015 at 11:02:55AM +0300, Tanu Kaskinen wrote:
> On Sun, 2015-09-20 at 23:39 +0200, Ahmed S. Darwish wrote:
> > diff --git a/src/pulse/context.c b/src/pulse/context.c
> > index ede01fa..f272cd6 100644
> > --- a/src/pulse/context.c
> > +++ b/src/pulse/context.c
> > @@ -591,6 +591,8 @@ static void setup_context(pa_context *c, pa_iochannel *io) {
> >      pa_tagstruct_putu32(t, PA_PROTOCOL_VERSION | (c->do_shm ? 0x80000000U : 0));
> >      pa_tagstruct_put_arbitrary(t, cookie, sizeof(cookie));
> >  
> > +    pa_tagstruct_put_boolean(t, pa_memfd_is_supported());
> 
> This breaks compatibility with old servers that expect the tagstruct to
> end after the cookie.
> 
> The "memfd supported" bit could be added to the version field like the
> "shm supported" bit.
>

Unfortunately this will also break compatibility with old servers.

Right now, PA uses the client's version number most-significant bit as
a marker for SHM support.

Afterwards, the server clears _only_ that bit:

    if (c->version >= 13) {
        shm_on_remote = !!(c->version & 0x80000000U);
        c->version &= 0x7FFFFFFFU;
    }

So if I overload any of the bits above, e.g. the second MSB bit
0x40000000, "old" servers will not clear it and will interpret the
client version as 0x4000001F instead of just v31 0x1F :-(

> Another approach would be to do add a new command
> to do memfd negotiation before enabling srbchannel. Abusing the version
> field is ugly, adding another negotiation adds more connection setup
> overhead. I don't know which approach we should choose.
>

Can we agree that _from this point forward_, the server clears the
client's version most-significant _16_ bits? This will give us 15
free new flags to play with.

I personally prefer this simple version flag solution since things
like xdg-app are not mainstream yet. It will also prepare us better
for the old-server/new-client container scenarios of the future.

But .. no hard opinions. If this is not accepted, I'll resort to
adding a new context state between PA_CONTEXT_GETTING_NAME and
PA_CONTEXT_READY for memfd negotiations.

Regards,

-- 
Darwish
http://darwish.chasingpointers.com