After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 774819 - alsasink: do wait in write() only if really needed
alsasink: do wait in write() only if really needed
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-11-21 22:30 UTC by Petr Kulhavy
Modified: 2016-11-23 13:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to do wait only on -EAGAIN (2.22 KB, patch)
2016-11-21 23:40 UTC, Petr Kulhavy
none Details | Review

Description Petr Kulhavy 2016-11-21 22:30:35 UTC
gst_alsasink_write() *always* calls snd_pcm_wait() before writing to ALSA. In fact the wait is needed only if there is not enough space to write the data. Since ALSA is open in non-blocking mode snd_pcm_writei() can be called first hoping there is enough space and wait done only if it returns -EAGAIN.

Doing wait only if needed should reduce the overall number of syscalls and improve performance at high packet rate.
Comment 1 Tim-Philipp Müller 2016-11-21 22:36:29 UTC
Are you going to work on a patch?
Comment 2 Petr Kulhavy 2016-11-21 23:40:11 UTC
Created attachment 340480 [details] [review]
Patch to do wait only on -EAGAIN

Here is the patch. 

I hope it makes sense what I'm saying. I haven't done any comparative performance test of the two approaches but it seems plausible to me. In a good case it would be just 1x write (ioctl), in a bad case it would be 1x wait (poll) and 2x write (ioctl) - but it would have to wait anyway...

The bad case happens when the audio chunk arrives too early or the buffer is overrunning (i.e. the decoder clock is slower than the encoder).

The other approach would be to use blocking mode. That would actually save syscalls (the wait would be eliminated). What was the reason to use non-blocking?
Comment 3 Jan Schmidt 2016-11-22 01:35:13 UTC
non-blocking was from bug #559111 to handle unplugging of USB devices better
Comment 4 Petr Kulhavy 2016-11-23 12:20:29 UTC
I've done some tracing on a live stream and it turned out that the first write systematically returns -EAGAIN on every call of alsasink_write(). 

This would be OK if the source was e.g. a file, but on a live stream I can't explain this behaviour. I would expect that the -EAGAIN would be more or less stochastic, depending on when the audio packet arrives.

So under these circumstances it makes no sense to apply my patch. It makes it actually worse, instead of 2 syscalls there are always 3 :-)
Comment 5 Jan Schmidt 2016-11-23 12:45:43 UTC
That behaviour makes sense to me:

With alsasink, there's the hardware ringbuffer (200ms by default, or whatever the hardware can support), and the GStreamer ringbuffer (200ms + 1 segment by default). The writing thread transfers buffers to ALSA as fast as it can, and is allowed to write at 1-segment-per-segment-output-by-ALSA. If the GStreamer ringbuffer is empty, it'll write a segment of silence - but a live stream has to run at least 200ms late before the GStreamer ringbuffer can be empty and cause that to happen.
Comment 6 Petr Kulhavy 2016-11-23 13:04:59 UTC
Ah, now I see. I was assuming that the ALSA buffer can run also partially empty.
So my change makes no sense in this situation. Sorry for the noise.