Re: [pulseaudio-discuss] [PATCH] tunnel-{sink, source}-new: Fix assertion when used with module-loopback
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. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tunnel-{sink, source}-new: Fix assertion when used with module-loopback
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. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tunnel-{sink, source}-new: Fix assertion when used with module-loopback
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 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tunnel-{sink, source}-new: Fix assertion when used with module-loopback
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. 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). ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tunnel-{sink, source}-new: Fix assertion when used with module-loopback
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. -- Tanu https://www.patreon.com/tanuk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tunnel-{sink, source}-new: Fix assertion when used with module-loopback
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. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tunnel-{sink, source}-new: Fix assertion when used with module-loopback
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". -- Tanu https://www.patreon.com/tanuk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss