Re: [pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
On 17.02.2018 22:05, Raman Shuishniou wrote: 17.02.2018 15:31, Georg Chini пишет: On 17.02.2018 11:08, Raman Shuishniou wrote: 16.02.2018 23:50, Georg Chini пишет: On 16.02.2018 17:37, Raman Shishniou wrote: On 02/16/2018 12:00 PM, Georg Chini wrote: On 14.02.2018 23:16, Raman Shyshniou wrote: Currently the pipe-source does not produce any data if no writer is connected. This patch enable silence generator when last writer closed pipe. It will stop automatically when any data appears. --- To me it appears like the only correct solution is to implement some local buffering, so that we can rewind the source and drop the remaining silence when pipe data comes in. It looks rather difficult to implement silence generation correctly, maybe we should drop the whole idea and stick with your original suspend/unsuspend approach. Implementing some local buffering would however increase underrun stability (at least when used with module-loopback). What do you think? I think we don't need to implement local buffering only to make a correct switching between silence and pipe. Fifo assumes local writer and the buffering should be implemented on it's side if it's necessary for underrun stability. Suspend/resume algorithm will be enough to not confuse source-outputs when no writers connected to pipe. I'll try to reimplement original autosuspend patch to make it more platform-independed. OK. One more point I have been thinking about: The pipe sets POLLIN as soon as data is available, which means we will possibly run an iteration of the thread function every couple of samples, leading to high CPU load, depending on the way the writer delivers the data. It could be changed to a timer based approach - there are the pa_smoother functions which account for the difference between system clock and writer clock. (See for example the alsa-sink code, there we have a similar situation.) So if we stick to the idea of generating silence, it might be worth considering a timer based approach. The writer also should use the CPU too much to write a couple of samples at each iteration. I don't think we need to worry about this in pipe-source module. If the writer writes larger chunks of data to the pipe, the calculation of the latency should be reconsidered. Currently it returns the amount of data in the pipe. This means, the latency will remain constant for some time and then suddenly jump up, when the next block is written. Would it not be better to return the time since the last pipe read as latency? You can actually test what is better by observing the end-to-end latency that module-loopback reports. The numbers should not vary too much after a minute of runtime - around +/-500 usec is a normal value if you set adjust_time=1. If you are seeing jumps of several ms, the source does not report latency correctly. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
17.02.2018 15:31, Georg Chini пишет: On 17.02.2018 11:08, Raman Shuishniou wrote: 16.02.2018 23:50, Georg Chini пишет: On 16.02.2018 17:37, Raman Shishniou wrote: On 02/16/2018 12:00 PM, Georg Chini wrote: On 14.02.2018 23:16, Raman Shyshniou wrote: Currently the pipe-source does not produce any data if no writer is connected. This patch enable silence generator when last writer closed pipe. It will stop automatically when any data appears. --- I agree that the timer can be disabled and that the time stamp gets meaningless the way it is set now when reading from the pipe. But we must continue without interruption when we switch from pipe to silence, so we should set the time stamp to something meaningful before switching to silence. (Switching between pipe and silence properly would not have worked with my code above.) Switching from pipe to silence can be easily done by generation a little less silence in first iteration. Yes, that is what the pseudo code is doing. It's not working when switching from silence to pipe. Actually we don't know if the writer can buffer data and/or trying to do some congession control. When the pipe have some data - source must read and post it immediately. While switching from silence to pipe we can wait until we run out of silence, but pipe has data to post right here and right now. For example, if the latency is 20ms and we wait for 10ms before starting the first read, the writer will just write more data to pipe while we waiting (+10ms). We will read and post that extra data on the next iteration. So we need to drop extra data from pipe or buffer the head of data each iteration. Both solutions are bad. I think we'll just give the source-output(s) a chance to deal with the extra silence we generated. You are (again) right, waiting for the silence to be played only moves the problem to the first pipe read. What happens when pipe data is coming in is that the latency (as seen by the source output) suddenly jumps up by possibly as much as one full source latency. If the receiving side does not compensate somehow (like module-loopback does), switching between pipe and silence multiple times will lead to accumulated latency. Therefore I do not think it is acceptable to hope that the source-output can deal with the extra silence. The accumulated latency can be decreased when the writer disconnects. To me it appears like the only correct solution is to implement some local buffering, so that we can rewind the source and drop the remaining silence when pipe data comes in. It looks rather difficult to implement silence generation correctly, maybe we should drop the whole idea and stick with your original suspend/unsuspend approach. Implementing some local buffering would however increase underrun stability (at least when used with module-loopback). What do you think? I think we don't need to implement local buffering only to make a correct switching between silence and pipe. Fifo assumes local writer and the buffering should be implemented on it's side if it's necessary for underrun stability. Suspend/resume algorithm will be enough to not confuse source-outputs when no writers connected to pipe. I'll try to reimplement original autosuspend patch to make it more platform-independed. One more point I have been thinking about: The pipe sets POLLIN as soon as data is available, which means we will possibly run an iteration of the thread function every couple of samples, leading to high CPU load, depending on the way the writer delivers the data. It could be changed to a timer based approach - there are the pa_smoother functions which account for the difference between system clock and writer clock. (See for example the alsa-sink code, there we have a similar situation.) So if we stick to the idea of generating silence, it might be worth considering a timer based approach. The writer also should use the CPU too much to write a couple of samples at each iteration. I don't think we need to worry about this in pipe-source module. -- Raman ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
On 17.02.2018 11:08, Raman Shuishniou wrote: 16.02.2018 23:50, Georg Chini пишет: On 16.02.2018 17:37, Raman Shishniou wrote: On 02/16/2018 12:00 PM, Georg Chini wrote: On 14.02.2018 23:16, Raman Shyshniou wrote: Currently the pipe-source does not produce any data if no writer is connected. This patch enable silence generator when last writer closed pipe. It will stop automatically when any data appears. --- I agree that the timer can be disabled and that the time stamp gets meaningless the way it is set now when reading from the pipe. But we must continue without interruption when we switch from pipe to silence, so we should set the time stamp to something meaningful before switching to silence. (Switching between pipe and silence properly would not have worked with my code above.) Switching from pipe to silence can be easily done by generation a little less silence in first iteration. Yes, that is what the pseudo code is doing. It's not working when switching from silence to pipe. Actually we don't know if the writer can buffer data and/or trying to do some congession control. When the pipe have some data - source must read and post it immediately. While switching from silence to pipe we can wait until we run out of silence, but pipe has data to post right here and right now. For example, if the latency is 20ms and we wait for 10ms before starting the first read, the writer will just write more data to pipe while we waiting (+10ms). We will read and post that extra data on the next iteration. So we need to drop extra data from pipe or buffer the head of data each iteration. Both solutions are bad. I think we'll just give the source-output(s) a chance to deal with the extra silence we generated. You are (again) right, waiting for the silence to be played only moves the problem to the first pipe read. What happens when pipe data is coming in is that the latency (as seen by the source output) suddenly jumps up by possibly as much as one full source latency. If the receiving side does not compensate somehow (like module-loopback does), switching between pipe and silence multiple times will lead to accumulated latency. Therefore I do not think it is acceptable to hope that the source-output can deal with the extra silence. To me it appears like the only correct solution is to implement some local buffering, so that we can rewind the source and drop the remaining silence when pipe data comes in. It looks rather difficult to implement silence generation correctly, maybe we should drop the whole idea and stick with your original suspend/unsuspend approach. Implementing some local buffering would however increase underrun stability (at least when used with module-loopback). What do you think? One more point I have been thinking about: The pipe sets POLLIN as soon as data is available, which means we will possibly run an iteration of the thread function every couple of samples, leading to high CPU load, depending on the way the writer delivers the data. It could be changed to a timer based approach - there are the pa_smoother functions which account for the difference between system clock and writer clock. (See for example the alsa-sink code, there we have a similar situation.) So if we stick to the idea of generating silence, it might be worth considering a timer based approach. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
16.02.2018 23:50, Georg Chini пишет: On 16.02.2018 17:37, Raman Shishniou wrote: On 02/16/2018 12:00 PM, Georg Chini wrote: On 14.02.2018 23:16, Raman Shyshniou wrote: Currently the pipe-source does not produce any data if no writer is connected. This patch enable silence generator when last writer closed pipe. It will stop automatically when any data appears. --- After my fixes to module-null-source, I think your logic is not yet completely correct. I would propose to do it like that: In source_process_msg(): case PA_SOURCE_MESSAGE_GET_LATENCY: current_latency = now - time stamp if (u->corkfd < 0) current_latency += data in pipe case PA_SOURCE_MESSAGE_SET_STATE: if (SUSPENDED or INIT -> IDLE || SUSPENDED or INIT -> RUNNING) { get time stamp u->starting = true } In thread_func(): close u->corkfd u->corkfd = -1 u->starting = true timer_elapsed = false revents = 0 get time stamp for (;;) { if (source is open) { /* We have to wait at least one configured source latency before starting * to read data */ if (revents & POLLIN && !u->starting) { read data from pipe if (u->corkfd >=0) { close corkfd u->corkfd = -1 } } else if (timer_elapsed && u->corkfd > 0) generate silence if (data was read/generated) { post data time stamp += data written } set timer absolute time stamp + configured (or fixed) source latency } else set timer disabled run rtpoll get timer_elapsed if (u->starting && timer_elapsed) u->starting = false if (revents & POLLHUP) { open pipe for writing u->corkfd = write file descriptor revents = revents & ~POLLHUP } error check } You can also add a source_update_requested_latency_cb() like in module-null-source and pass PA_SOURCE_DYNAMIC_LATENCY to pa_source_new() to make the latency configurable. I hope I did not forget anything ... The incoming data can has a sample rate that differs from the system clock. For example, due to imperfect hardware oscillator. It's a bad idea to expect a data at the system clock rate. When the source is receiving a real data from pipe the timer should be disabled and u->timestamp and u->starting doesn't make sense. You don't rely on the timer when data is coming from the pipe. The rtpollrun will return as soon as data is available in the pipe and then the timer expiration time will be moved forward. You are right, if the clocks do not match, the timer will expire sooner or later, before data is available in the pipe. This does not matter however because there is no data available and corkfd is < 0, it will just be an iteration without any action. You can anyway not expect that the thread is only woken up when the timer expires or something happens on the pipe side. The thread is also woken up when messages are sent to it. I agree that the timer can be disabled and that the time stamp gets meaningless the way it is set now when reading from the pipe. But we must continue without interruption when we switch from pipe to silence, so we should set the time stamp to something meaningful before switching to silence. (Switching between pipe and silence properly would not have worked with my code above.) Switching from pipe to silence can be easily done by generation a little less silence in first iteration. It's not working when switching from silence to pipe. Actually we don't know if the writer can buffer data and/or trying to do some congession control. When the pipe have some data - source must read and post it immediately. While switching from silence to pipe we can wait until we run out of silence, but pipe has data to post right here and right now. For example, if the latency is 20ms and we wait for 10ms before starting the first read, the writer will just write more data to pipe while we waiting (+10ms). We will read and post that extra data on the next iteration. So we need to drop extra data from pipe or buffer the head of data each iteration. Both solutions are bad. I think we'll just give the source-output(s) a chance to deal with the extra silence we generated. Also I still believe that waiting one pipe fill time before reading the first data from the pipe will increase stability because then there is a larger buffer. When switching between pipe and silence, we have to wait until pipe data or silence runs out, otherwise we will supply too many samples. I have made another attempt on some pseudo-code, hope it is better this time: for (;;) { if (source is open) { /* We wait one configured source latency before starting * to read data to improve stability */ if (revents & POLLIN && !u->st
Re: [pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
On 16.02.2018 17:37, Raman Shishniou wrote: On 02/16/2018 12:00 PM, Georg Chini wrote: On 14.02.2018 23:16, Raman Shyshniou wrote: Currently the pipe-source does not produce any data if no writer is connected. This patch enable silence generator when last writer closed pipe. It will stop automatically when any data appears. --- After my fixes to module-null-source, I think your logic is not yet completely correct. I would propose to do it like that: In source_process_msg(): case PA_SOURCE_MESSAGE_GET_LATENCY: current_latency = now - time stamp if (u->corkfd < 0) current_latency += data in pipe case PA_SOURCE_MESSAGE_SET_STATE: if (SUSPENDED or INIT -> IDLE || SUSPENDED or INIT -> RUNNING) { get time stamp u->starting = true } In thread_func(): close u->corkfd u->corkfd = -1 u->starting = true timer_elapsed = false revents = 0 get time stamp for (;;) { if (source is open) { /* We have to wait at least one configured source latency before starting * to read data */ if (revents & POLLIN && !u->starting) { read data from pipe if (u->corkfd >=0) { close corkfd u->corkfd = -1 } } else if (timer_elapsed && u->corkfd > 0) generate silence if (data was read/generated) { post data time stamp += data written } set timer absolute time stamp + configured (or fixed) source latency } else set timer disabled run rtpoll get timer_elapsed if (u->starting && timer_elapsed) u->starting = false if (revents & POLLHUP) { open pipe for writing u->corkfd = write file descriptor revents = revents & ~POLLHUP } error check } You can also add a source_update_requested_latency_cb() like in module-null-source and pass PA_SOURCE_DYNAMIC_LATENCY to pa_source_new() to make the latency configurable. I hope I did not forget anything ... The incoming data can has a sample rate that differs from the system clock. For example, due to imperfect hardware oscillator. It's a bad idea to expect a data at the system clock rate. When the source is receiving a real data from pipe the timer should be disabled and u->timestamp and u->starting doesn't make sense. You don't rely on the timer when data is coming from the pipe. The rtpollrun will return as soon as data is available in the pipe and then the timer expiration time will be moved forward. You are right, if the clocks do not match, the timer will expire sooner or later, before data is available in the pipe. This does not matter however because there is no data available and corkfd is < 0, it will just be an iteration without any action. You can anyway not expect that the thread is only woken up when the timer expires or something happens on the pipe side. The thread is also woken up when messages are sent to it. I agree that the timer can be disabled and that the time stamp gets meaningless the way it is set now when reading from the pipe. But we must continue without interruption when we switch from pipe to silence, so we should set the time stamp to something meaningful before switching to silence. (Switching between pipe and silence properly would not have worked with my code above.) Also I still believe that waiting one pipe fill time before reading the first data from the pipe will increase stability because then there is a larger buffer. When switching between pipe and silence, we have to wait until pipe data or silence runs out, otherwise we will supply too many samples. I have made another attempt on some pseudo-code, hope it is better this time: for (;;) { if (source is open) { /* We wait one configured source latency before starting * to read data to improve stability */ if (revents & POLLIN && !u->starting) { if (u->corkfd >=0) { close corkfd u->corkfd = -1 /* Let's wait until we run out of silence before * sending pipe data */ u->starting = true } else { read data from pipe /* This is the time when we will run out of data */ time stamp = now + data to write } } else if (timer_elapsed && u->corkfd >= 0) { generate silence (amount now - timestamp) time stamp += data to write } if (data was read/generated) post data if (starting || u->corkfd >= 0) set timer absolute time stamp + fixed source latency else set timer disabled } else set timer disabled run rtpoll get timer_elapsed if (u->starting && timer_elapsed) u->starting = false if (revents & POLLHUP && u-
Re: [pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
On 02/16/2018 12:00 PM, Georg Chini wrote: > On 14.02.2018 23:16, Raman Shyshniou wrote: >> Currently the pipe-source does not produce any data if no >> writer is connected. This patch enable silence generator >> when last writer closed pipe. It will stop automatically >> when any data appears. >> --- > After my fixes to module-null-source, I think your logic is not yet > completely correct. I would propose to do it like that: > > In source_process_msg(): > > case PA_SOURCE_MESSAGE_GET_LATENCY: >current_latency = now - time stamp >if (u->corkfd < 0) >current_latency += data in pipe > > case PA_SOURCE_MESSAGE_SET_STATE: >if (SUSPENDED or INIT -> IDLE || SUSPENDED or INIT -> RUNNING) > { >get time stamp >u->starting = true >} > > In thread_func(): > > close u->corkfd > u->corkfd = -1 > u->starting = true > > timer_elapsed = false > revents = 0 > get time stamp > > for (;;) { > if (source is open) { > > /* We have to wait at least one configured source latency before > starting > * to read data */ > if (revents & POLLIN && !u->starting) { > read data from pipe > if (u->corkfd >=0) { > close corkfd > u->corkfd = -1 >} > > } else if (timer_elapsed && u->corkfd > 0) > generate silence > >if (data was read/generated) { > post data > time stamp += data written >} > >set timer absolute time stamp + configured (or fixed) source latency >} else >set timer disabled > >run rtpoll >get timer_elapsed > > if (u->starting && timer_elapsed) > u->starting = false > >if (revents & POLLHUP) { >open pipe for writing >u->corkfd = write file descriptor >revents = revents & ~POLLHUP >} > >error check > } > > You can also add a source_update_requested_latency_cb() like > in module-null-source and pass PA_SOURCE_DYNAMIC_LATENCY > to pa_source_new() to make the latency configurable. > > I hope I did not forget anything ... The incoming data can has a sample rate that differs from the system clock. For example, due to imperfect hardware oscillator. It's a bad idea to expect a data at the system clock rate. When the source is receiving a real data from pipe the timer should be disabled and u->timestamp and u->starting doesn't make sense. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
On 16.02.2018 13:20, Georg Chini wrote: On 16.02.2018 12:35, Raman Shishniou wrote: On 02/16/2018 12:00 PM, Georg Chini wrote: On 14.02.2018 23:16, Raman Shyshniou wrote: Currently the pipe-source does not produce any data if no writer is connected. This patch enable silence generator when last writer closed pipe. It will stop automatically when any data appears. --- After my fixes to module-null-source, I think your logic is not yet completely correct. I would propose to do it like that: In source_process_msg(): case PA_SOURCE_MESSAGE_GET_LATENCY: current_latency = now - time stamp if (u->corkfd < 0) current_latency += data in pipe case PA_SOURCE_MESSAGE_SET_STATE: if (SUSPENDED or INIT -> IDLE || SUSPENDED or INIT -> RUNNING) { get time stamp u->starting = true } In thread_func(): close u->corkfd u->corkfd = -1 u->starting = true timer_elapsed = false revents = 0 get time stamp for (;;) { if (source is open) { /* We have to wait at least one configured source latency before starting * to read data */ if (revents & POLLIN && !u->starting) { read data from pipe if (u->corkfd >=0) { close corkfd u->corkfd = -1 } } else if (timer_elapsed && u->corkfd > 0) generate silence if (data was read/generated) { post data time stamp += data written } set timer absolute time stamp + configured (or fixed) source latency } else set timer disabled run rtpoll get timer_elapsed if (u->starting && timer_elapsed) u->starting = false if (revents & POLLHUP) { open pipe for writing u->corkfd = write file descriptor revents = revents & ~POLLHUP } Unfortunately not all platforms generate POLLHUP when writer closed a pipe: Moreover, some platforms generate POLLIN with or without POLLHUP https://www.greenend.org.uk/rjk/tech/poll.html So, the only correct way is try to read EOF (0) from pipe when POLLIN or POLLHUP in revents. To me it looks like all relevant platforms set POLLHUP (and maybe additionally POLLIN). So I would be fine with a POLLHUP check, but if you think you have to take the other platforms into account, I don't mind. error check } You can also add a source_update_requested_latency_cb() like in module-null-source and pass PA_SOURCE_DYNAMIC_LATENCY to pa_source_new() to make the latency configurable. The pipe-source can change latency only when it generating a silence. I don't think so. Basically, what is done by the pseudo-code above is to wait one latency and only then start reading. So you can specify whatever latency you like, as long as the writer (not the pipe itself) is able to buffer the data. Just to make it clear - I am fine with keeping a fixed latency (what the pipe can buffer). It was only an idea. Maybe it is not good to rely on the writer to buffer data. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
On 16.02.2018 12:35, Raman Shishniou wrote: On 02/16/2018 12:00 PM, Georg Chini wrote: On 14.02.2018 23:16, Raman Shyshniou wrote: Currently the pipe-source does not produce any data if no writer is connected. This patch enable silence generator when last writer closed pipe. It will stop automatically when any data appears. --- After my fixes to module-null-source, I think your logic is not yet completely correct. I would propose to do it like that: In source_process_msg(): case PA_SOURCE_MESSAGE_GET_LATENCY: current_latency = now - time stamp if (u->corkfd < 0) current_latency += data in pipe case PA_SOURCE_MESSAGE_SET_STATE: if (SUSPENDED or INIT -> IDLE || SUSPENDED or INIT -> RUNNING) { get time stamp u->starting = true } In thread_func(): close u->corkfd u->corkfd = -1 u->starting = true timer_elapsed = false revents = 0 get time stamp for (;;) { if (source is open) { /* We have to wait at least one configured source latency before starting * to read data */ if (revents & POLLIN && !u->starting) { read data from pipe if (u->corkfd >=0) { close corkfd u->corkfd = -1 } } else if (timer_elapsed && u->corkfd > 0) generate silence if (data was read/generated) { post data time stamp += data written } set timer absolute time stamp + configured (or fixed) source latency } else set timer disabled run rtpoll get timer_elapsed if (u->starting && timer_elapsed) u->starting = false if (revents & POLLHUP) { open pipe for writing u->corkfd = write file descriptor revents = revents & ~POLLHUP } Unfortunately not all platforms generate POLLHUP when writer closed a pipe: Moreover, some platforms generate POLLIN with or without POLLHUP https://www.greenend.org.uk/rjk/tech/poll.html So, the only correct way is try to read EOF (0) from pipe when POLLIN or POLLHUP in revents. To me it looks like all relevant platforms set POLLHUP (and maybe additionally POLLIN). So I would be fine with a POLLHUP check, but if you think you have to take the other platforms into account, I don't mind. error check } You can also add a source_update_requested_latency_cb() like in module-null-source and pass PA_SOURCE_DYNAMIC_LATENCY to pa_source_new() to make the latency configurable. The pipe-source can change latency only when it generating a silence. I don't think so. Basically, what is done by the pseudo-code above is to wait one latency and only then start reading. So you can specify whatever latency you like, as long as the writer (not the pipe itself) is able to buffer the data. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
On 02/16/2018 12:00 PM, Georg Chini wrote: > On 14.02.2018 23:16, Raman Shyshniou wrote: >> Currently the pipe-source does not produce any data if no >> writer is connected. This patch enable silence generator >> when last writer closed pipe. It will stop automatically >> when any data appears. >> --- > After my fixes to module-null-source, I think your logic is not yet > completely correct. I would propose to do it like that: > > In source_process_msg(): > > case PA_SOURCE_MESSAGE_GET_LATENCY: >current_latency = now - time stamp >if (u->corkfd < 0) >current_latency += data in pipe > > case PA_SOURCE_MESSAGE_SET_STATE: >if (SUSPENDED or INIT -> IDLE || SUSPENDED or INIT -> RUNNING) > { >get time stamp >u->starting = true >} > > In thread_func(): > > close u->corkfd > u->corkfd = -1 > u->starting = true > > timer_elapsed = false > revents = 0 > get time stamp > > for (;;) { > if (source is open) { > > /* We have to wait at least one configured source latency before > starting > * to read data */ > if (revents & POLLIN && !u->starting) { > read data from pipe > if (u->corkfd >=0) { > close corkfd > u->corkfd = -1 >} > > } else if (timer_elapsed && u->corkfd > 0) > generate silence > >if (data was read/generated) { > post data > time stamp += data written >} > >set timer absolute time stamp + configured (or fixed) source latency >} else >set timer disabled > >run rtpoll >get timer_elapsed > > if (u->starting && timer_elapsed) > u->starting = false > >if (revents & POLLHUP) { >open pipe for writing >u->corkfd = write file descriptor >revents = revents & ~POLLHUP >} > Unfortunately not all platforms generate POLLHUP when writer closed a pipe: Moreover, some platforms generate POLLIN with or without POLLHUP https://www.greenend.org.uk/rjk/tech/poll.html So, the only correct way is try to read EOF (0) from pipe when POLLIN or POLLHUP in revents. >error check > } > > You can also add a source_update_requested_latency_cb() like > in module-null-source and pass PA_SOURCE_DYNAMIC_LATENCY > to pa_source_new() to make the latency configurable. The pipe-source can change latency only when it generating a silence. > > I hope I did not forget anything ... ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
On 14.02.2018 23:16, Raman Shyshniou wrote: Currently the pipe-source does not produce any data if no writer is connected. This patch enable silence generator when last writer closed pipe. It will stop automatically when any data appears. --- After my fixes to module-null-source, I think your logic is not yet completely correct. I would propose to do it like that: In source_process_msg(): case PA_SOURCE_MESSAGE_GET_LATENCY: current_latency = now - time stamp if (u->corkfd < 0) current_latency += data in pipe case PA_SOURCE_MESSAGE_SET_STATE: if (SUSPENDED or INIT -> IDLE || SUSPENDED or INIT -> RUNNING) { get time stamp u->starting = true } In thread_func(): close u->corkfd u->corkfd = -1 u->starting = true timer_elapsed = false revents = 0 get time stamp for (;;) { if (source is open) { /* We have to wait at least one configured source latency before starting * to read data */ if (revents & POLLIN && !u->starting) { read data from pipe if (u->corkfd >=0) { close corkfd u->corkfd = -1 } } else if (timer_elapsed && u->corkfd > 0) generate silence if (data was read/generated) { post data time stamp += data written } set timer absolute time stamp + configured (or fixed) source latency } else set timer disabled run rtpoll get timer_elapsed if (u->starting && timer_elapsed) u->starting = false if (revents & POLLHUP) { open pipe for writing u->corkfd = write file descriptor revents = revents & ~POLLHUP } error check } You can also add a source_update_requested_latency_cb() like in module-null-source and pass PA_SOURCE_DYNAMIC_LATENCY to pa_source_new() to make the latency configurable. I hope I did not forget anything ... ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v6 1/2] pipe-source: generate silence when no writers connected
Currently the pipe-source does not produce any data if no writer is connected. This patch enable silence generator when last writer closed pipe. It will stop automatically when any data appears. --- src/modules/module-pipe-source.c | 113 ++- 1 file changed, 87 insertions(+), 26 deletions(-) diff --git a/src/modules/module-pipe-source.c b/src/modules/module-pipe-source.c index f8284c1..b7eb4bf 100644 --- a/src/modules/module-pipe-source.c +++ b/src/modules/module-pipe-source.c @@ -33,6 +33,7 @@ #include #endif +#include #include #include @@ -71,7 +72,11 @@ struct userdata { pa_thread_mq thread_mq; pa_rtpoll *rtpoll; +pa_usec_t timestamp; +pa_usec_t latency; + char *filename; +int corkfd; int fd; pa_memchunk memchunk; @@ -90,6 +95,7 @@ static const char* const valid_modargs[] = { NULL }; +/* Called from thread context */ static int source_process_msg( pa_msgobject *o, int code, @@ -101,17 +107,28 @@ static int source_process_msg( switch (code) { +case PA_SOURCE_MESSAGE_SET_STATE: + +if (u->corkfd >= 0 && PA_SOURCE_IS_OPENED(PA_PTR_TO_UINT(data))) +u->timestamp = pa_rtclock_now(); + +break; + case PA_SOURCE_MESSAGE_GET_LATENCY: { -size_t n = 0; +int64_t latency = 0; +if (u->corkfd >= 0) +latency = (int64_t)pa_rtclock_now() - (int64_t)u->timestamp; #ifdef FIONREAD -int l; +else { +int l; -if (ioctl(u->fd, FIONREAD, &l) >= 0 && l > 0) -n = (size_t) l; +if (ioctl(u->fd, FIONREAD, &l) >= 0 && l > 0) +latency = pa_bytes_to_usec((size_t) l, &u->source->sample_spec); +} #endif -*((int64_t*) data) = pa_bytes_to_usec(n, &u->source->sample_spec); +*((int64_t*) data) = latency; return 0; } } @@ -129,6 +146,10 @@ static void thread_func(void *userdata) { pa_thread_mq_install(&u->thread_mq); +/* Close our writer here to start silence generation or suspend source if no writers left */ +pa_assert_se(pa_close(u->corkfd) == 0); +u->corkfd = -1; + for (;;) { int ret; struct pollfd *pollfd; @@ -136,7 +157,7 @@ static void thread_func(void *userdata) { pollfd = pa_rtpoll_item_get_pollfd(u->rtpoll_item, NULL); /* Try to read some data and pass it on to the source driver */ -if (u->source->thread_info.state == PA_SOURCE_RUNNING && pollfd->revents) { +if (pollfd->revents) { ssize_t l; void *p; @@ -151,47 +172,78 @@ static void thread_func(void *userdata) { l = pa_read(u->fd, (uint8_t*) p + u->memchunk.index, pa_memblock_get_length(u->memchunk.memblock) - u->memchunk.index, &read_type); pa_memblock_release(u->memchunk.memblock); -pa_assert(l != 0); /* EOF cannot happen, since we opened the fifo for both reading and writing */ +if (PA_LIKELY(l > 0)) { +if (PA_SOURCE_IS_OPENED(u->source->thread_info.state)) { +u->memchunk.length = (size_t) l; +pa_source_post(u->source, &u->memchunk); +u->memchunk.index += (size_t) l; + +if (u->memchunk.index >= pa_memblock_get_length(u->memchunk.memblock)) { +pa_memblock_unref(u->memchunk.memblock); +pa_memchunk_reset(&u->memchunk); +} +} -if (l < 0) { +if (u->corkfd >= 0) { +pa_assert_se(pa_close(u->corkfd) == 0); +u->corkfd = -1; +pa_rtpoll_set_timer_disabled(u->rtpoll); +} +} else if (l == 0) { +if (u->corkfd < 0) { +pa_log_debug("There are no writers left"); + +if ((u->corkfd = pa_open_cloexec(u->filename, O_WRONLY, 0)) < 0) { +pa_log("open('%s'): %s", u->filename, pa_cstrerror(errno)); +goto fail; +} + +u->latency = pa_bytes_to_usec(pa_pipe_buf(u->fd), &u->source->sample_spec); +u->timestamp = pa_rtclock_now(); +} +} else { if (errno == EINTR) continue; else if (errno != EAGAIN) { pa_log("Failed to read data from FIFO: %s", pa_cstrerror(errno)); goto fail; } +} +} -} else { +if (u->corkfd >= 0 && PA_SOURCE_IS_OPENED(u->source->thread_info.state)) { +pa_usec_t now; +size_t l; -u->memchunk.length = (size_t) l; -pa_source_post(u->sourc