If the audio API takes a while for starting the audio callback, the
current heuristic can be off. In particular, with very short files, it
can happen that the audio callback is not called before playback is
stopped, so no audio is output at all.
Change draining so that it essentially waits for the ringbuffer to
empty. The assumption is that once the audio API has read the data
via the callback, it will always output it, even if the audio API
is stopped right after the callback has returned.
If a frame could only be partially filled with real audio data, the
silence wasn't written at the correct offset. It could have happened
that the remainder of the frame contained garbage.
(This didn't happen in the more common case of playing dummy silence.)
If the audio callback suddenly stops, and the AO provides no "reset"
callback, then reset() could deadlock by waiting on the audio callback
forever.
The waiting was needed to enter a consistent state, where the audio
callback guarantees it won't access the ringbuffer. This in turn is
needed because mp_ring_reset() is not concurrency-safe.
This active waiting is unavoidable. But the way it was implemented, the
audio callback had to call ao_read_data() at least once when reset() is
called. Fix this by making ao_read_data() set a flag upon entering and
leaving, which basically turns p->state into some sort of spinlock.
The audio callback actually never needs to spin, because there are only
2 states: playing audio, or playing silence. This might be a bit
surprising, because usually atomic_compare_exchange_strong() requires a
retry-loop idiom for correct operation.
This commit is needed because ao_wasapi can (or will in the future)
randomly stop the audio callback in certain corner cases. Then the
player would hang forever in reset().
With --gapless-audio=no, changing from one file to the next apparently
made it hang, until the player was woken up by unrelated events like
input. The reason was that the AO doesn't notify the player of EOF
properly. the played was querying ao_eof_reached(), and then just went
to sleep, without anything waking it up.
Make it event-based: the AO wakes up the playloop if the EOF state
changes.
We could have fixed this in a simpler way by synchronously draining the
AO in these cases. But I think proper event handling is preferable.
Fixes: #1069
CC: @mpv-player/stable (perhaps)
bstr.c doesn't really deserve its own directory, and compat had just
a few files, most of which may as well be in osdep. There isn't really
any justification for these extra directories, so get rid of them.
The compat/libav.h was empty - just delete it. We changed our approach
to API compatibility, and will likely not need it anymore.
Logic for this was missing from pull.c. For push.c it was missing if the
driver didn't support it. But even if the driver supported it (such as
with ao_alsa), strange behavior was observed by users. See issue #933.
Always check explicitly whether the AO is in paused mode, and if so,
don't drain.
Possibly fixes#933.
CC: @mpv-player/stable
There were subtle and minor race conditions in the pull.c code, and AOs
using it (jack, portaudio, sdl, wasapi). Attempt to remove these.
There was at least a race condition in the ao_reset() implementation:
mp_ring_reset() was called concurrently to the audio callback. While the
ringbuffer uses atomics to allow concurrent access, the reset function
wasn't concurrency-safe (and can't easily be made to).
Fix this by stopping the audio callback before doing a reset. After
that, we can do anything without needing synchronization. The callback
is resumed when resuming playback at a later point.
Don't call driver->pause, and make driver->resume and driver->reset
start/stop the audio callback. In the initial state, the audio callback
must be disabled.
JackAudio of course is different. Maybe there is no way to suspend the
audio callback without "disconnecting" it (what jack_deactivate() would
do), so I'm not trying my luck, and implemented a really bad hack doing
active waiting until we get the audio callback into a state where it
won't interfere. Once the callback goes from AO_STATE_WAIT to NONE, we
can be sure that the callback doesn't access the ringbuffer or anything
else anymore. Since both sched_yield() and pthread_yield() apparently
are not always available, use mp_sleep_us(1) to avoid burning CPU during
active waiting.
The ao_jack.c change also removes a race condition: apparently we didn't
initialize _all_ ao fields before starting the audio callback.
In ao_wasapi.c, I'm not sure whether reset really waits for the audio
callback to return. Kovensky says it's not guaranteed, so disable the
reset callback - for now the behavior of ao_wasapi.c is like with
ao_jack.c, and active waiting is used to deal with the audio callback.
In my opinion, we shouldn't use atomics at all, but ok.
This switches the mpv code to use C11 stdatomic.h, and for compilers
that don't support stdatomic.h yet, we emulate the subset used by mpv
using the builtins commonly provided by gcc and clang.
This supersedes an earlier similar attempt by Kovensky. That attempt
unfortunately relied on a big copypasted freebsd header (which also
depended on much more highly compiler-specific functionality, defined
reserved symbols, etc.), so it had to be NIH'ed.
Some issues:
- C11 says default initialization of atomics "produces a valid state",
but it's not sure whether the stored value is really 0. But we rely on
this.
- I'm pretty sure our use of the __atomic... builtins is/was incorrect.
We don't use atomic load/store intrinsics, and access stuff directly.
- Our wrapper actually does stricter typechecking than the stdatomic.h
implementation by gcc 4.9. We make the atomic types incompatible with
normal types by wrapping them into structs. (The FreeBSD wrapper does
the same.)
- I couldn't test on MinGW.
Same change as in e2184fcb, but this time for pull based AOs. This is
slightly controversial, because it will make a fast syscall from e.g.
ao_jack. And according to JackAudio developers, syscalls are evil and
will destroy realtime operation. But I don't think this is an issue at
all.
Still avoid locking a mutex. I'm not sure what jackaudio does in the
worst case - but if they set the jackaudio thread (and only this thread)
to realtime, we might run into deadlock situations due to priority
inversion and such. I'm not quite sure whether this can happen, but I'll
readily follow the cargo cult if it makes hack happy.
Until now, this was always conflated with uninit. This was ugly, and
also many AOs emulated this manually (or just ignored it). Make draining
an explicit operation, so AOs which support it can provide it, and for
all others generic code will emulate it.
For ao_wasapi, we keep it simple and basically disable the internal
draining implementation (maybe it should be restored later).
Tested on Linux only.
This has 2 goals:
- Ensure that AOs have always enough data, even if the device buffers
are very small.
- Reduce complexity in some AOs, which do their own buffering.
One disadvantage is that performance is slightly reduced due to more
copying.
Implementation-wise, we don't change ao.c much, and instead "redirect"
the driver's callback to an API wrapper in push.c.
Additionally, we add code for dealing with AOs that have a pull API.
These AOs usually do their own buffering (jack, coreaudio, portaudio),
and adding a thread is basically a waste. The code in pull.c manages
a ringbuffer, and allows callback-based AOs to read data directly.
