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

2017-08-12 Thread 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.
___
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

2017-07-24 Thread Georg Chini

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

2017-07-24 Thread 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
___
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

2017-07-22 Thread Georg Chini

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

2017-07-20 Thread Tanu Kaskinen
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

2017-07-17 Thread Georg Chini

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

2017-07-17 Thread Tanu Kaskinen
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