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

Tanu Kaskinen tanuk at iki.fi
Tue Jan 12 23:00:58 PST 2016


On Wed, 2016-01-13 at 00:19 +0200, Ahmed S. Darwish wrote:
> 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 :-(

That should be ok, since 0x4000001F will be larger than the old
server's version, so the negotiated version will be the old server's
version. The same thing happens when you have version 12 server and
version 13 client with shm support. The server doesn't clear the MSB,
so it thinks the client has very large version.

> > 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 can agree to that. 16 bits should still be plenty for the version
number.

-- 
Tanu


More information about the pulseaudio-discuss mailing list