GNOME Bugzilla – Bug 702972
audioringbuffer: gst_audio_ring_buffer_prepare_read() might read partially written segment
Last modified: 2018-05-06 10:31:20 UTC
Created attachment 247632 [details] [review] Patch gst_audio_ring_buffer_prepare_read() reads per segment whereas default_commit() might not filled totally the segment depending on the sample offset for instance. This means that gst_audio_ring_buffer_prepare_read() might read a partially written segment. In the patch in attachment, I added a "writepointer" which makes it possible to calculate the difference between the last written segment and the last read segment in gst_audio_ring_buffer_prepare_read(). If the difference is lower than 1 the function sets the readpointer (readptr) to the previous segment read and decrement segdone as it should be incremented later on by the function calling gst_audio_ring_buffer_prepare_read(). This adds an extra latency as there will be always at least one full segment in the audioringbuffer and there might be a more optimize way to handle such a case. Please comments if I am saying something wrong :)
That would mean that a segment is read twice in the worst case if the writer thread is too slow with filling the segments? (Also this breaks ABI, you should decrement the size of the gst_reserved pointer in the GstAudioRingBuffer struct and move the segwritten field right above it)
Created attachment 248127 [details] [review] Patch
Apparently I did not post my last comment :s just provided a patch with a mistake... Actually, it could be more than twice for instance when retrieving audio from the network with jitter. In any case, when reading more than once, it should read a cleared segment thus a silence (as done in gstaudiosink). The third patch provided, add the forgotten modification in gstaudiosrc and correct the ABI (thx Sebastian). Thx to let me know if I am missing something there.
Created attachment 248155 [details] [review] Patch
But wouldn't that mean that the source is too slow to fill the ringbuffer anyway, and realtime playback will never work properly? Also I think your patch causes A/V sync to get lost. The idea is that the ringbuffer is always filled faster than it is read, and if that doesn't happen for a short moment the reader of the ringbuffer will shortly read old values from the ringbuffer, but later the writer will catch up again.
Paul?
Closing this bug report as no further information has been provided. Please feel free to reopen this bug report if you can provide the information that was asked for in a previous comment. Thanks!