[pulseaudio-discuss] [PATCH] tunnel-{sink, source}-new: Fix assertion when used with module-loopback

[Tanu Kaskinen]

On Sat, 2017-07-22 at 10:21 +0200, Georg Chini wrote:
> On 20.07.2017 15:48, Tanu Kaskinen wrote:
> > On Mon, 2017-07-17 at 19:53 +0200, Georg Chini wrote:
> > > On 17.07.2017 19:32, Tanu Kaskinen wrote:
> > > > On Sun, 2017-07-16 at 11:42 +0200, Georg Chini wrote:
> > > > > Currently pulseaudio crashes with an assertion in pa_rtpoll_item_new_asyncmsgq_read()
> > > > > if a loopback is applied to a tunnel-new sink/source because tunnel-{sink,source}-new
> > > > > do not set thread_info.rtpoll. Bug was reported on IRC.
> > > > > 
> > > > > This patch fixes the problem by initializing thread_info.rtpoll properly.
> > > > 
> > > > Did you test this patch? The tunnel devices don't run the rtpoll that
> > > > you create, so I would expect that the loopback won't work.
> > > > 
> > > > This is a known bug that I started working on in the past:
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=73429
> > > > 
> > > > I made some patches, issues got pointed out in review, and then I never
> > > > finished v2 of the patches. I haven't given up on that, but it's been a
> > > > year since I last worked on it... In case you're interested in the
> > > > current state of the v2 patches, I pushed the code to
> > > > git://people.freedesktop.org/~tanuk/pulseaudio branch "rtpoll-mainloop-
> > > > v2".
> > > > 
> > > 
> > > I haven't tested it myself, but the bug reporter on IRC (tar-dingens)
> > > tested them and said it worked. The patch looked like the easiest way to
> > > make module-loopback happy.
> > 
> > I don't understand how it can work. module-loopback uses the rtpoll to
> > set up the internal asyncmsgq. If the tunnel device doesn't run the
> > rtpoll, the messages shouldn't get processed. One of the messages is
> > used to send memchunks from the source to the sink, so there should be
> > no audio moving if the messages aren't processed.
> > 
> 
> I think I know why it works. The messages are processed in
> sink_input_pop_cb(). module-loopback calls
> pa_asyncmsgq_process_one() from there. So it is not necessary
> to run the rtpoll at all. From what I know about the loopback code
> it should be fine this way.

Yep.

> The question is, if we can hit other issues with the "fake" rtpoll.
> If not, I think we could generally go with the patch (and add some
> comments in the tunnel modules that the rtpoll is not used at all
> and only there to make module-loopback happy).

I think this is ok as a temporary band-aid. At least module-combine-
sink and module-rtp-recv are affected too, and they might not work with
this patch, but since their previous behaviour was to crash, it's hard
to imagine how they could work any worse than that.

-- 
Tanu

https://www.patreon.com/tanuk