1654151 - After cubeb-coreaudio-rs PR #99, the audio position sometimes jumps forward by 11ms

Status: RESOLVED DUPLICATE of bug 1649859

(Core :: Audio/Video: Playback, defect)

Description

[Markus Stange [:mstange]]

Here's a profile taken in a test build that includes the audio position interpolation fix from PR #99 (see bug 1649859): https://share.firefox.dev/3hhHlMf

You can see that most VideoFrameTimeStampJitter values are fine, but there is a jump by around 11ms at some point. In practice, such a jump happens around once every 10 seconds or so.

This happens because stm.output_callback_tstamp and stm.frames_played are updated separately during the callback. If position() is called between the two writes, then the output time is already updated but the frames_played still have the old value, and the returned position is off by the amount of time that passes between the audio callback invocations.

Comment 1

[Markus Stange [:mstange]]

I have experimented with a fix that groups output_callback_tstamp and frames_played into a struct, and uses the triple_buffer crate to update the combined value atomically without blocking. (A mutex would be simpler, but then we'd introduce the possibility of blocking inside the audio callback.)
The experimental fix solves the problem completely. Here's a profile of 90 seconds without a single jitter instance that even comes close to a millisecond: https://share.firefox.dev/3eIbHWl

I've put the patch here, but it's not very readable because it includes a small refactoring inside the output callback function: https://gist.github.com/mstange/8a14e1b6413ceffc3eea4e1675e9caaf

Comment 2

[Paul Adenot (:padenot)]

Ok thanks I'll get this fixed upstream. Unfortunately, triple-buffer doesn't work because cubeb_stream_get_position can be called by any thread (i.e. not just a single consumer). It's sufficient to show the problem and hint at a solution however.

Comment 3

[Markus Stange [:mstange]]

(In reply to Paul Adenot (:padenot) from comment #2)

cubeb_stream_get_position can be called by any thread

Surely not on multiple threads at the same time, though? position takes &mut self, and creating a &mut is only legal if the reference is truly an exclusive reference. Is cubeb really playing it that fast and loose with the meaning of exclusive references?

If it actually is an exclusive reference, i.e. if ownership of the AudioUnitStream object is properly passed between threads between calls to position(), then using triple_buffer should be fine because triple_buffer::Output is Send.

Comment 4

[Paul Adenot (:padenot)]

(In reply to Markus Stange [:mstange] from comment #3)

Is cubeb really playing it that fast and loose with the meaning of exclusive references?

Probably, for now. In practice, it's always called from the same thread however.

Comment 5

[Markus Stange [:mstange]]

My suggested fix landed as part of PR #99.