On 21.02.2018 12:50, Raman Shishniou wrote:
On 02/21/2018 02:24 PM, Georg Chini wrote:
On 21.02.2018 12:22, Raman Shishniou wrote:
On 02/21/2018 12:13 PM, Raman Shishniou wrote:
On 02/21/2018 09:39 AM, Georg Chini wrote:
On 21.02.2018 06:05, Georg Chini wrote:
On 21.02.2018 05:55, Georg Chini wrote:
On 20.02.2018 22:34, Raman Shishniou wrote:
On 02/20/2018 11:04 PM, Georg Chini wrote:
On 20.02.2018 19:49, Raman Shishniou wrote:
On 02/20/2018 07:02 PM, Georg Chini wrote:
On 20.02.2018 16:38, Raman Shyshniou wrote:
Currently the pipe-source will remain running even if no
writer is connected and therefore no data is produced.
This patch adds the autosuspend=<bool> option to prevent this.
Source will stay suspended if no writer is connected.
This option is enabled by default.
---
src/modules/module-pipe-source.c | 279
+++++++++++++++++++++++++++++----------
1 file changed, 212 insertions(+), 67 deletions(-)
I think I need post a simple pseudo code of new thread loop because it
was completely rewritten. There are too many changes in one patch.
It can be difficult to see the whole picture of new main loop.
Well, I applied the patch and looked at the result. I still don't like the
approach.
I would propose this:
auto_suspended = false;
revents = 0
events = POLLIN
for (;;) {
/* This is the part that is run when the source is opened
* or auto suspended
if (SOURCE_IS_OPENED(source) || auto_suspended) {
/* Check if we wake up from user suspend */
if (corkfd >= 0 && !auto_suspended) {
len = 0
close pipe for writing
}
/* We received POLLIN or POLLHUP or both */
if (revents) {
/* Read data from pipe */
len = read data
/* Got data, post it */
if (len > 0) {
if (auto_suspend) {
send unsuspend message
auto_suspend = false
}
post data
We cannot post data here because source still suspended. Sending resume message
is not enough
to immediately resume the source. We need to wait several poll runs until it
will be resumed.
(source->thread_info.state changed in this thread, i.e. during poll run). But
we will see
POLLIN and/or POLLHUP each run if we don't remove pipe fd from polling.
Why do we have to wait? The source will be unsuspended on the next rtpollrun.
I do not see why we cannot already push data. Or does something get lost?
Why would we receive POLLIN on each run? We read the data from the pipe.
If you think the data should not be posted, you can just skip posting and
discard
the data. According to your pseudo-code it is done like tis in your previous
patch.
I should not write mails before I have woken up completely ... I see what you
mean
now (and I also see that you do not discard data as I thought). But I still
believe you
can post the data before the source gets unsuspended. What is the difference if
the
samples are stored in the source or in the source output? Anyway we are talking
about a time frame of (very probably) less than 1 ms between sending the message
and receiving it. To ensure that the loop works as expected, auto_suspended
should
be set/reset in the suspend/unsuspend message and not directly in the thread
function.
POLLHUP spam cannot happen because corkfd will be opened on the first POLLHUP.
POLLIN spam cannot happen when auto_suspend is set/reset from the message
handler.
Not, I can't post it here. The source may not be resumed at all after we send a
resume message.
Not within 1 ms, not within next hour. It can be autosuspended and suspended by
user manually
after it. I that case we read data and should discard it instead of posting (as
you propose).
But that algorithm will post data to suspended source while it suspended by
user.
Also auto_suspended can't be set/reset in suspend/resume message handler
because it called from
main context and accessed from thread context.
That's why I read data and wait while source will be resumed before posting.
I just looked into pa_source_post() code:
void pa_source_post(pa_source*s, const pa_memchunk *chunk) {
pa_source_output *o;
void *state = NULL;
pa_source_assert_ref(s);
pa_source_assert_io_context(s);
pa_assert(PA_SOURCE_IS_LINKED(s->thread_info.state));
pa_assert(chunk);
if (s->thread_info.state == PA_SOURCE_SUSPENDED)
return;
...
There are only 3 valid states of source to post data:
static inline bool PA_SOURCE_IS_LINKED(pa_source_state_t x) {
return x == PA_SOURCE_RUNNING || x == PA_SOURCE_IDLE || x ==
PA_SOURCE_SUSPENDED;
}
And if the source is suspended:
if (s->thread_info.state == PA_SOURCE_SUSPENDED)
return;
If we read some data, send resume and try to post, chunk will be just discarded
in pa_source_post().
So we must to wait source->thread_info.state will be changed to RUNNING or IDLE
before posting any data. And the only way to wait - call some pa_rtpoll_run()
and check the source state to be valid for posting after each call. Again,
we must stop polling pipe while we waiting because we can get endless loop
if source stays suspended for long time after we send a resume message.
I think my algorithm implemented in this patch is the simplest way to achieve
this.
Well, your code is not doing the right thing either. When the source gets user
suspended, there will be some (trailing) data you read from the pipe. Now you
use this data as an indicator, that the source got suspended. When the source
gets unsuspended, the first thing you do is post the trailing data that was read
when the source was suspended. And only after that you start polling the pipe
again
I can't track the suspend reason in i/o thread right now. It's not copied to
thread_info in pa_source struct along with state during state changes.
Tanu proposed a patches that will pass pa_suspend_cause_t to
SINK/SOURCE_SET_STATE
handlers and set_state() callbacks. It we add suspend_cause to thread_info too,
there will be easy way to discard data if we are suspended by user:
if (PA_SOURCE_IS_OPENED(u->source->thread_info.state)) {
... post data ...
chunk.length = 0;
} else if (PA_SUSPEND_APPLICATION is not in thread_info->suspend_cause) {
... discard data ...
chunk.length = 0;
}
I see another problem. If, during suspend, a writer connects and
disconnects again, the pipe may be full of old data after we resume.
So I guess we have to read data from the pipe continuously and
discard it while the source is suspended.
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
