[pulseaudio-discuss] [PATCH v2 09/12] pulsecore: pstream: Support memfd blocks transport

[Ahmed S. Darwish]

Hi!

On Tue, Feb 23, 2016 at 01:34:41PM +0100, David Henningsson wrote:
> 
> On 2016-02-20 01:19, Ahmed S. Darwish wrote:
> >
...
> >
> >Well, the reason is that we want to close the memfd fd directly
> >after sending the packet and its ancillary using POSIX sendmsg().
> >pa_pstream_send_tagstruct_**() does not offer us any ability to
> >do that :-( It _opaques the message_ then defers the write event
> >over the pstream.
> >
> >Due to such opaqueness, after a succesful pstream do_write() I
> >cannot just say: the packet written was a PA_COMMAND_SMID_FD, so
> >let's close its associated fds.
> 
> It seems quite straight forward just to add another parameter "bool
> close_fds" to pa_pstream_send_tagstruct_with_fds and struct
> pa_cmsg_ancil_data, and in pstream.c::do_write, right after the call to
> pa_iochannel_write_with_fds, you close the fds if the parameter is set?
> 
> That way you don't need any "cleanup hook" either.
>

That's infinitely better, and honestly what should've been done
in the first place..

Will now update v3 to use a PA_COMMAND and this form of direct,
parameter-based, cleanup.

> >>
> >>As a side note, we could potentially optimise setting up an srbchannel by
> >>sending all three memfds in PA_COMMAND_ENABLE_SRBCHANNEL, it'll be one less
> >>package to send...
> >>
> >
> >I'm really not a big fan of this.. It  will hardcode both the
> >number, _and nature_, of mempools in the Pulse communication
> >protocol itslef. Are you sure we want to do that?
> 
> Hey, it was just an idea :-)
> 
> Indeed that would mean some additional code, probably not worth it
> at this point. There are just these really quick clients, e g if you
> do "pactl list sinks" or something, and we're adding package after
> package just to set the connection up. But thinking about it, this
> would probably be better fixed if, e g, "pactl" could just disable
> SHM in the first place.
>

Thanks for your understanding :-)

Regards,
Darwish