Set pcm state to SND_PCM_STATE_XRUN in case -EPIPE is received,
and handle this state as per the usual logic.
This way snd_pcm_prepare gets called, and the loop continued.
Inspired by a patch posted by malc_ on #mpv.
--audio-stream-silence is a shitty feature compensating for awful
consumer garbage, that mutes PCM at first to check whether it's
compressed audio, using formats advocated and owned by malicious patent
troll companies (who spend more money on their lawyers than paying any
technicians), wrapped in a wasteful way to make it constant bitrate
using a standard whose text is not freely available, and only rude users
want it. This feature has been carelessly broken, because it's
complicated and stupid. What would Jesus do? If not getting an aneurysm,
or pushing over tables with expensive A/V receivers on top of them, he'd
probably fix the feature. So let's take inspiration from Jesus Christ
himself, and do something as dumb as wasting some of our limited
lifetime on this incredibly stupid fucking shit.
This is tricky, because state changes like end-of-audio are supposed to
be driven by the AO driver, while playing silence precludes this. But it
seems code paths for "untimed" AOs can be reused.
But there are still problems. For example, underruns will just happen
normally (and stop audio streaming), because we don't have a separate
heuristic to check whether the buffer is "low enough" (as a consequence
of a network stall, but before the audio output itself underruns).
Create a central function which pumps data through the filter. This also
might fix bogus use of the filter API on flushing. (The filter is just
used for convenience, but I guess the overall result is still simpler.)
It is now the AO's responsibility to handle period size alignment. The
ao->period_size alignment field is unused as of the recent audio
refactor commit. Remove it.
It turns out that ao_alsa shows extremely inefficient behavior as a
consequence of the removal of period size aligned writes in the
mentioned refactor commit. This is because it could get into a state
where it repeatedly wrote single samples (as small as 1 sample), and
starved the rest of the player as a consequence. Too bad. Explicitly
align the size in ao_alsa. Other AOs, which need this, should do the
same.
One reason why it broke so badly with ao_alsa was that it retried the
write() even if all reported space could be written. So stop doing that
too. Retry the write only if we somehow wrote less.
I'm not sure about ao_pulse.
This replaces the two buffers (ao_chain.ao_buffer in the core, and
buffer_state.buffers in the AO) with a single queue. Instead of having a
byte based buffer, the queue is simply a list of audio frames, as output
by the decoder. This should make dataflow simpler and reduce copying.
It also attempts to simplify fill_audio_out_buffers(), the function I
always hated most, because it's full of subtle and buggy logic.
Unfortunately, I got assaulted by corner cases, dumb features (attempt
at seamless looping, really?), and other crap, so it got pretty
complicated again. fill_audio_out_buffers() is still full of subtle and
buggy logic. Maybe it got worse. On the other hand, maybe there really
is some progress. Who knows.
Originally, the data flow parts was meant to be in f_output_chain, but
due to tricky interactions with the playloop code, it's now in the dummy
filter in audio.c.
At least this improves the way the audio PTS is passed to the encoder in
encoding mode. Now it attempts to pass frames directly, along with the
pts, which should minimize timestamp problems. But to be honest, encoder
mode is one big kludge that shouldn't exist in this way.
This commit should be considered pre-alpha code. There are lots of bugs
still hiding.
Previously get_state() would keep setting the cork status
while paused, but it only does for that after underflows now.
Correct this oversight by creating the stream corked for start()
to uncork it at a later time.
fixes#8026
FFmpeg expects those fields to be set on the AVFrame when
encoding audio, not doing so will cause the avcodec_send_frame
call to return EINVAL (at least in recent builds).
When get_state() corks the stream after an underrun happens
priv->playing is incorrectly reset to true, which can cause the
player to miss the underrun entirely. Stop resetting priv->playing
during corking (but not uncorking) to fix this.
The underflow callback introduced in d27ad96 can be called
when the buffer is still full, causing playback to never
resume afterwards since get_state() reports free_samples == 0.
Fix this by fully resetting on underrun, which flushes
the stream and ensures free buffer space.
fixes#7874
The pull mode APIs were previously required to have thread-safe
ao_controls. However, locks were added in b83bdd1 for parity with push
mode. This introduced deadlocks in ao_wasapi.
Instead, only lock ao_control for the push mode APIs.
fixes#7787
See also #7832, #7811. We'll wait for feedback to see if those should
also be closed.
AOs which use the "push" API must set this field now. Actually, this was
sort of always required, but happened to work anyway. The future
intention is to use device_buffer as the pre-buffer amount, which has to
be available right before audio playback is started. "Pull" AOs really
need this too conceptually, just that the API is underspecified.
From what I can see, only ao_null did not do this yet.
Previously, device_buffer defaulted to 0 on pulse. This meant that
commit baa7b5c would always wait with a timeout of 0, leading to
high CPU usage for PulseAudio users.
By setting device_buffer to the number of samples per channel that
PulseAudio sets as its target, this commit fixes this behaviour.
The playback thread may obviously still fill the AO'S entire audio
buffer, which means it unset p->draining, which makes no sense and broke
ao_drain(). So just don't unset it here.
Not sure if this really fixes this, it was hard to reproduce. Regression
due to the recent changes. There are probably many more bugs like this.
Stupid asynchronous nightmare state machine. Give me a language that
supports formal verification (in presence of concurrency) or something.
I feel like this makes slightly more sense. At least it doesn't include
the potentially arbitrary constant latency that is generally included in
the delay value. Also, the buffer status doesn't matter - either we've
filled the entire buffer (then we can wait this long), or there's not
enough data anyway (then the core will wake up the thread if new data is
available).
But ultimately, we have to guess, unless the AO does notify us with
ao_wakeup_playthread().
Draining may now wait for no reason up to 1/4th of the total buffer
time. Shouldn't be a disimprovement in practice.
It's conceivable that ao->driver->reset() will make the audio API wait
for ao_read_data() (i.e. its audio callback) to return. Since we
recently moved the reset() call inside the same lock that ao_read_data()
acquires, this could deadlock. Whether this really happens depends on
how exactly the AO behaves. For example, ao_wasapi does not have this
problem. "Push" AOs are not affected either.
Fix by moving it outside of the lock. Assume ao->driver->start() will
not have this problem.
Could affect ao_sdl, ao_coreaudio (and similar rotten fruit AOs). I'm
unsure whether anyone experienced the problem in practice.
Instead of the relatively subtle underflow handling, simply signal
whether the stream is in a playing state. Should make it more robust.
Should affect ao_alsa and ao_pulse only (and ao_openal, but it's
broken).
For ao_pulse, I'm just guessing. How the hell do you query whether a
stream is playing? Who knows. Seems to work, judging from very
superficial testing.
Just a detail. If wrong (not unlikely because I'm just guessing my own
messy state machine), this will make the player freeze due to waiting
for something that never happens. Enjoy.
The feeder thread basically woke up the core and itself too often, and
caused some CPU overhead. This was caused by the recent buffer.c
changes.
For one, do not let ao_read_data() wake up the core, and instead rely on
the feeder thread's own buffer management. This is a bit strange, since
the change intended to unify the buffer management, but being more
consequent about it is better deferred to later, when the buffer
management changes again anyway. And also, the "more" condition in the
feeder thread seems outdated, or at least what made it make sense has
been destroyed, so do something that may or may not be better. In any
case, I'm still not getting underruns with ao_alsa, but the wakeup
hammering is gone.
This affects "pull" AOs only: ao_alsa, ao_pulse, ao_openal, ao_pcm,
ao_lavc. There are changes to the other AOs too, but that's only about
renaming ao_driver.resume to ao_driver.start.
ao_openal is broken because I didn't manage to fix it, so it exits with
an error message. If you want it, why don't _you_ put effort into it? I
see no reason to waste my own precious lifetime over this (I realize the
irony).
ao_alsa loses the poll() mechanism, but it was mostly broken and didn't
really do what it was supposed to. There doesn't seem to be anything in
the ALSA API to watch the playback status without polling (unless you
want to use raw UNIX signals).
No idea if ao_pulse is correct, or whether it's subtly broken now. There
is no documentation, so I can't tell what is correct, without reverse
engineering the whole project. I recommend using ALSA.
This was supposed to be just a simple fix, but somehow it expanded scope
like a train wreck. Very high chance of regressions, but probably only
for the AOs listed above. The rest you can figure out from reading the
diff.
wasapi/coreaudio/sdl were affected, alsa/pusle were not.
The confusion here was that resume() has different meaning with pull and
push AOs.
Fixes: #7772
Regression since the recent refactor. How did nobody notice?
This happened because the push code now calls the function for the pull
code. Both the former and latter apply the volume, so oops.
The recent change to the common code removed all calls to ->drain. It's
currently emulated via a timed sleep and polling ao_eof_reached(). That
is actually fallback code for AOs which lacked draining. I could just
readd the drain call, but it was a bad idea anyway. My plan to handle
this better is to require the AO to signal a underrun, even if
AOPLAY_FINAL_CHUNK is not set. Also reinstate not possibly waiting for
ao_lavc.c. ao_pcm.c did not have anything to handle this; whatever.
This is preparation to further cleanups (and eventually actual
improvements) of the audio output code.
AOs are split into two classes: pull and push. Pull AOs let an audio
callback of the native audio API read from a ring buffer. Push AOs
expose a function that works similar to write(), and for which we start
a "feeder" thread. It seems making this split was beneficial, because of
the different data flow, and emulating the one or other in the AOs
directly would have created code duplication (all the "pull" AOs had
their own ring buffer implementation before it was cleaned up).
Unfortunately, both types had completely separate implementations (in
pull.c and push.c). The idea was that little can be shared anyway. But
that's very annoying now, because I want to change the API between AO
and player.
This commit attempts to merge them. I've moved everything from push.c to
pull.c, the trivial entrypoints from ao.c to pull.c, and attempted to
reconcile the differences. It's a mess, but at least there's only one
ring buffer within the AO code now. Everything should work mostly the
same. Pull AOs now always copy the audio data under a lock; before this
commit, all ring buffer access was lock-free (except for the decoder
wakeup callback, which acquired a mutex). In theory, this is "bad", and
people obsessed with lock-free stuff will hate me, but in practice
probably won't matter. The planned change will probably remove this
copying-under-lock again, but who knows when this will happen.
One change for the push AOs now makes it drop audio, where before only a
warning was logged. This is only in case of AOs or drivers which exhibit
unexpected (and now unsupported) behavior.
This is a risky change. Although it's completely trivial conceptually,
there are too many special cases. In addition, I barely tested it, and
I've messed with it in a half-motivated state over a longer time, barely
making any progress, and finishing it under a rush when I already should
have been asleep. Most things seem to work, and I made superficial tests
with alsa, sdl, and encode mode. This should cover most things, but
there are a lot of tricky things that received no coverage. All this
text means you should be prepared to roll back to an older commit and
report your problem.
Ancient Linux audio output. Apparently it survived until now, because
some BSDs (but not all) had use of this. But these should work with
ao_sdl or ao_openal too (that's why these AOs exist after all). ao_oss
itself has the problem that it's virtually unmaintainable from my point
of view due to all the subtle (or non-subtle) difference. Look at the
ifdef mess and the multiple code paths (that shouldn't exist) in the
removed source code.
I wonder what this even is. I've never heard of anyone using it, and
can't find a corresponding library that actually builds with it. Good
enough to remove.
It was always marked as "experimental", and had inherent problems that
were never fixed. It was disabled by default, and I don't think anyone
is using it.
Looks like the recent change to this actually made it crash whenever
audio happened to be initialized first, due to not setting the
mux_stream field before the on_ready callback. Mess a way around this.
Also remove a stray unused variable from ao_lavc.c.
In shared mode, we previously tried to feed the full native format to
IsFormatSupported in the hopes that the "closest match" returned was
actually that.
Turns out, IsFormatSupported will always return the mix format if we
don't use the mix format's sample rate. This will also clobber our
choice of channel map with the mix format channel map even if our
desired channel map is supported due to surround emulation.
The solution is to not bother trying to use anything other than the mix
format sample rate. While we're at it, we might as well use the mix
format PCM sample format (always float32) since this conversion will
happen anyway and may avoid unecessary dithering to intermediate
integer formats if we are already resampling or channel mixing.
Change all OPT_* macros such that they don't define the entire m_option
initializer, and instead expand only to a part of it, which sets certain
fields. This requires changing almost every option declaration, because
they all use these macros. A declaration now always starts with
{"name", ...
followed by designated initializers only (possibly wrapped in macros).
The OPT_* macros now initialize the .offset and .type fields only,
sometimes also .priv and others.
I think this change makes the option macros less tricky. The old code
had to stuff everything into macro arguments (and attempted to allow
setting arbitrary fields by letting the user pass designated
initializers in the vararg parts). Some of this was made messy due to
C99 and C11 not allowing 0-sized varargs with ',' removal. It's also
possible that this change is pointless, other than cosmetic preferences.
Not too happy about some things. For example, the OPT_CHOICE()
indentation I applied looks a bit ugly.
Much of this change was done with regex search&replace, but some places
required manual editing. In particular, code in "obscure" areas (which I
didn't include in compilation) might be broken now.
In wayland_common.c the author of some option declarations confused the
flags parameter with the default value (though the default value was
also properly set below). I fixed this with this change.
This seems to be an older bug. It set priv->outputfilename to a new
talloc-allocated string, but the field is also managed as string option,
so talloc will free it first, then m_option_free() is called on the
dangling pointer. Possibly this is caused by the earlier ta destruction
order change.
Before this commit, option declarations used M_OPT_MIN/M_OPT_MAX (and
some other identifiers based on these) to signal whether an option had
min/max values. Remove these flags, and make it use a range implicitly
on the condition if min<max is true.
This requires care in all cases when only M_OPT_MIN or M_OPT_MAX were
set (instead of both). Generally, the commit replaces all these
instances with using DBL_MAX/DBL_MIN for the "unset" part of the range.
This also happens to fix some cases where you could pass over-large
values to integer options, which were silently truncated, but now cause
an error.
This commit has some higher potential for regressions.
Move the "old" mostly command line parsing and option management related
code to m_config_frontend.c/h. Move the the code that enables other part
of the player to access options to m_config_core.c/h. "frontend" is out
of lack of creativity for a better name.
Unfortunately, the separation isn't quite clean yet. m_config_frontend.c
still references some m_config_core.c implementation details, and
m_config_new() is even left in m_config_core.c for now. There some odd
functions that should be removed as well (marked as "Bad functions").
Fixing these things requires more changes and will be done separately.
struct m_config is left with the current name to reduce diff noise.
Also, since there are a _lot_ source files that include m_config.h, add
a replacement m_config.h that "redirects" to m_config_core.h.
A previous commit moved the underrun reporting to report_underruns(),
and called it from get_space(). One reason was that I worried about
printing a log message from a "realtime" callback, so I tried to move it
out of the way. (Though there's little justification other than a bad
feeling. While an older version of the pull code tried to avoid any
mutexes at all in the callback to accommodate "requirements" from APIs
like jackaudio, we gave up on that. Nobody has complained yet.)
Simplify this and move underrun reporting back to the callback. But
instead of printing the message from there, move the message into the
playloop. Change the message slightly, because ao->log is inaccessible,
and without the log prefix (e.g. "[ao/alsa]"), some context is missing.
AOs can report audio underruns, but only ao_alsa and ao_sdl (???)
currently do so. If the AO was marked as not reporting it, the cache
state was used to determine whether playback was interrupted due to slow
input.
This caused problems in some cases, such as video with very low video
frame rate: when a new frame is displayed, a new frame has to be
decoded, and since there it's so much further into the file (long frame
durations), the cache gets into an underrun state for a short moment,
even though both audio and video are playing fine. Enlarging the audio
buffer didn't help.
Fix this by making all AOs report underruns. If the AO driver does not
report underruns, fall back to using the buffer state.
pull.c behavior is slightly changed. Pull AOs are normally intended to
be used by pseudo-realtime audio APIs that fetch an audio buffer from
the API user via callback. I think it makes no sense to consider a
buffer underflow not an underrun in any situation, since we return
silence to the reader. (OK, maybe the reader could check the return
value? But let's not go there as long as there's no implementation.)
Remove the flag from ao_sdl.c, since it just worked via the generic
mechanism. Make the redundant underrun message verbose only.
push.c seems to log a redundant underflow message when resuming (because
somehow ao_play_data() is called when there's still no new data in the
buffer). But since ao_alsa does its own underrun reporting, and I only
use ao_alsa, I don't really care.
Also in all my tests, there seemed to be a rather high delay until the
underflow was logged (with audio only). I have no idea why this happened
and didn't try to debug this, but there's probably something wrong
somewhere.
This commit may cause random regressions.
See: #7440
If ao_add_events() is used, but all events flags are already set, then
we don't need to wakeup the core again.
Also, make the underrun message "exact" by avoiding the race condition
mentioned in the comment.
Avoiding redundant wakeups is not really worth the trouble, and it's
actually just a bonus in the change making the ao_underrun_event()
function return whether a new underrun was set, which is needed by the
following commit.
Jan Ekström
sfan5
wm4
LAGonauta
ekisu
Kevin Mitchell
Nicolas F
Rafael Rivera