On 22.02.2018 22:01, Raman Shishniou wrote:
On 02/22/2018 10:18 PM, Georg Chini wrote:
-        /* Hmm, nothing to do. Let's sleep */
-        pollfd->events = (short) (u->source->thread_info.state == 
+        /* Post data to source, discard data or wait for state transition to 
be complete */
+        if (PA_SOURCE_IS_OPENED(u->source->thread_info.state)) {
+            if (u->writer_connected && u->corkfd >= 0) {
+                pa_assert_se(pa_close(u->corkfd) == 0);
+                u->corkfd = -1;
+            }
+            if (u->memchunk.length > 0) {
+                pa_source_post(u->source, &u->memchunk);
+                pa_memblock_unref(u->memchunk.memblock);
+                u->memchunk.memblock = pa_memblock_new(u->core->mempool, 
+                u->memchunk.length = 0;
+            }
+        } else if (u->suspended_by_user)
You have to open the corkfd here as well to avoid POLLHUP spam if the writer
disconnects during user suspend.

During user suspend we are doing normal polling. If the writer will be 
we'll see it by reading 0 bytes and corkfd will be opened.

See below, I would like to avoid continuous polling during user suspend.

+            u->memchunk.length = 0;
+        pollfd->events = u->memchunk.length ? 0 : POLLIN;
How do you wake up from autosuspend? When the writer disconnects,
memchunk.length will be 0 and so you do no longer listen for POLLIN.
I guess you need to know the current suspend cause here:
        > user suspended -> events = 0
(only auto suspended + no writer connected) or running -> events = POLLIN
only auto suspended + writer connected (=wake up transition) -> events = 0

Yes. This is what I do, keep data in memchunk and wait while source will be 

Yeah, looks I am too dump to understand a simple comparison ... sorry. (Shame on me, I know I have been making a lot of silly mistakes during my reviews and your
knowledge of PA is better than mine. I hope I did not annoy you too much.)

But now I have another issue:
You are polling the pipe and running the loop even if the source is user suspended.
This seems like a waste of CPU (even more than accepting some POLLIN spam
during wakeup transition). I know you do it to discard data that is written during suspend, but I wonder if there is no better way to discard that data without running
the loop permanently.
I am thinking of draining the pipe in the SET_STATE handler. If you are setting
events = 0 and open the corkfd on user suspend, nothing except messages
should wake us up. Now, when the state changes to running, you can drain the
pipe in the SET_STATE handler. The thread loop will just run through on the first
iteration after user suspend, because revents = 0 and chunk.length = 0. Now,
because the source is open again, you can set events = POLLIN.
After that, you are back to normal.
You can safely assume writer_connected=false during user suspend (you do
not notice when the writer disconnects if you set events = 0). If the writer
is still connected after suspend, writer_connected will get set when you read the first data. It will cause an unnecessary unsuspend message, but this will
have no effect because neither the suspend cause nor the state change.

I would also suggest to use a flag like set_pollin in the comparison, set and reset the flag in the appropriate places and explain why in a comment. This is one of the situations, where a little bit more code could make the concept clearer. I don't
mind keeping it as is however, if you think it's not worth the effort.

pulseaudio-discuss mailing list

Reply via email to