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

[Georg Chini]

On 24.07.2017 21:35, Georg Chini wrote:
> On 24.07.2017 18:58, Tanu Kaskinen wrote:
>> 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.
>>
> I'll send a new version with clear comments then. But before I do
> so, I will test it myself and also check at least the behavior of
> combine-sink.
Luckily, combine-sink also works with the patch. Don't know how
to test module-rtp-recv.