Commit 10915000 attempted to fix wasting CPU when resyncing and no new
data was actually coming from the demuxer. The fix assumed that at this
point it would have reached the sync point, but since the code attempts
weird incremental decoding, this wasn't actually true. So it broke
seeking in addition to removing the CPU waste.
Try something else. This time, we essentially only wakeup again if
data was read (i.e. audio_decode() returned successfully).
Thsi code path happens during seeking. If video is still being decoded
to get to the first video frame, audio has nothing to do, as it is
synchronized against the first video frame. We only want to wake up if
there's an actual state change.
Fixes#1958.
The af_add() function has a problem: if the inserted filter returns
AF_DETACH during init, the function will have a dangling pointer. Until
now this was avoided by making sure none of the used filters actually
return AF_DETACH, but it's getting infeasible.
Solve this by requiring passing an unique label to af_add(), which is
then used instead of the pointer.
Only reinit filters if it's actually needed. This is also slightly
easier to understand: if you look at the code, it should now be more
obvious why a reinit is needed (hopefully).
Precise seeking requires skipping audio, since the demuxer usually
doesn't seek precisely enough. There is a sanity check that prevents
skipping more than 300 seconds of audio. This still fails with very
large mp3s. For example, with a 1GB sized mp3 with Xing headers, entries
will be 4 MB apart on average, and occasionally much more.
Just bump the limit. I'm not even sure why it was added in the first
place; I suppose it's most important for files with real PTS resets.
When playback is started after seeking or opening a file, we need to
make sure audio and video line up exactly. This is done by cutting or
padding the audio stream to start on the video PTS.
This does not quite work with spdif: audio is compressed data, within a
spdif frame. There is no way to cut the audio "in between" the frames.
Cutting between the frames would just produce broken spdif packets, and
who knows how receivers will react to this (play noise?). But we still
can cut it in frame boundaries.
Unfortunately, we also insert 0 data for "silence" - we probably
shouldn't do this. Chances are the receiver will switch to PCM or so.
But for now this will have to do.
Note that this could be simplified somewhat, as soon as we work with
frames. See previous commit.
Handle the failure gracefully, instead of exploding and disabling audio.
Just set the speed back to 1.0.
Also remove the AF_DETACH from af_scaletempo. This actually created a
dangling pointer in af_add(), a tricky consequence of af_add()
reconfiguring the filter chain and the newly added filter using
AF_DETACH. Fortunately the AF_DETACH is not needed (and probably never
worked - it comes from MPlayer times, and MPlayer also disables audio
when trying to change speed with spdif).
Always use af_scaletempo if it's inserted, even if the option
--audio-pitch-correction=no is set.
Make sure all filters are reset on speed change. It's conceivable that
dynamic changes to the filter chain at runtime leave filters around
without resetting their speed parameters.
Also move the code to a separate function.
If the audio decoder was created, but no audio filter chain created yet
(still trying to decode a first audio frame), setting the "speed"
property could explode. It tried to recreate the filter chain, even
though no format was set yet.
This is inconvenient and should not happen.
Although the libraries we use for resampling (libavresample and
libswresample) do not support changing sampelrate on the fly, this makes
it easier to make sure no audio buffers are implicitly dropped. In fact,
this commit adds additional code to drain the resampler explicitly.
Changing speed twice without feeding audio in-between made it crash
with libavresample inc ertain cases (libswresample is fine). This is
probably a libavresample bug. Hopefully this will be fixed, and also I
attempted to workaround the situation that crashes it. (It seems to
point in direction of random memory corruption, though.)
In my opinion the artifacts created by af_scaletempo on extreme slowdown
(50% or so) are too bothersome - but users disagree. So use
af_scaletempo on any speed changes, not just on speedup.
This avoids potentially dropping some small amount of audio data
buffered in filters.
Reinit can be skipped only if the filter is af_scaletempo (which maps to
AF_CONTROL_SET_PLAYBACK_SPEED). The other case using af_lavrresample is
much more complicated due to filter chain politics.
Also, changing speed between 1.0 and something higher typically inserts
or removes the filter, so this obviously requires reinitialization. It
can be prevented by forcing the filter with --af=scaletempo.
I guess this was supposed to be some sort of optimization, but even
though it probably works, it's pretty meaningless and I couldn't measure
a difference. One special case killed.
mpctx->audio_delay always has the same value as opts->audio_delay. (This
was not the case a long time ago, when the audio-delay property didn't
actually write to opts->audio_delay. I think.)
Some files can have audio after video has ended, and playback of the
audio-only remainder is supposed to work just fine.
Seeking is broken-ish though. Not much can be done about this, since
it's the way demuxers work. Also, such files are obscure corner cases.
But enabling hr-seek for audio after video end can improve the situation
a lot.
This helps with issue #1533. The reported also provided a command line
to produce such a file:
ffmpeg -i image.jpg -i audio.flac -threads $(nproc) \
-c:v libvpx -crf 10 -qmin 5 -qmax 55 \
-vf scale=360:-1 -sws_flags lanczos -c:a libvorbis -ac 2 \
-b:a 128K out.webm
This was forgotten when the option was implemented, and makes this
option work as advertised.
Fixes#1473 (though the default behavior is probably still stupid).
This is a somewhat obscure situation, and happens only if audio starts
again after it has ended (in particular can happens with files where
audio starts later). It doesn't matter much whether audio starts
immediately or some milliseconds later, so simplify it.
When playing paused, the amount of decoded audio is limited to a small
amount (1 sample), because we don't write any audio to the AO when
paused. The small amount could trigger the case of the wanted audio
being too far in the future in the PTS sync code, which set the audio
status to STATUS_DRAINING, which in turn triggered the EOF code in the
next iteration. This was ok, but unfortunately, this triggered another
retry in order to check resuming from EOF by setting the status to
STATUS_SYNCING, which in turn lead to the busy loop by alternating
between the 2 states. So don't try resyncing while paused.
Since the PTS syncing code also calls ao_reset(), this could cause the
pulseaudio daemon to consume some CPU time as well.
This was caused by commit 33b57f55. Before that, the playloop was merely
run more often, but didn't cause any problems.
Fixes#1288.
We absolutely need to clear the AO reference in the mixer.
The audio_status must be changed to a state where no code assumes that
the AO is available. (It's allowed to do this blindly.)
This rewrites the audio decode loop to some degree. Audio filters don't
do refcounted frames yet, so af.c contains a hacky "emulation".
Remove some of the weird heuristic-heavy code in dec_audio.c. Instead of
estimating how much audio we need to filter, we always filter full
frames. Maybe this should be adjusted later: in case filtering increases
the volume of the audio data, we should try not to buffer too much
filter output by reducing the input that is fed at once.
For ad_spdif.c and ad_mpg123.c, we don't avoid extra copying yet - it
doesn't seem worth the trouble.
Use a pseudo-filter when changing speed with resampling, instead of
somehow changing a samplerate somewhere. This uses the same underlying
mechanism, but is a bit more structured and cleaner. It also makes some
of the following changes easier.
Since we now always use filters to change audio speed, move most of the
work set_playback_speed() does to recreate_audio_filters().
This is what you would expect. Before this commit, each
ao_request_reload() call would just queue a reload command, and then
recreate the AO for the number of times the function was called.
Instead of sending a command, introduce some sort of event retrieval
mechanism. At least for the reload case, use atomics, because we're too
lazy to setup an extra mutex.
This commit fixes a "cosmetic" user interface issue. Instead of
displaying the interpolated seek time on OSD, show the actual audio
time.
This is rather silly: when seeking in audio-only mode, it takes some
iterations until audio is "ready", but on the other hand, the audio
state machine is rather fickle, and fixing this cosmetic issue would be
intrusive. So just add a hack that paints over the ugly behavior as
perceived by the user. Probably the lesser evil.
It doesn't happen if video is enabled, because that mode sets the
current time immediately to video PTS. (Audio has to be synced to video,
so the code is a bit more complex.)
Fixes#1233.
The player was supposed to exit playback if both video and audio failed
to initialize (or if one of the streams was not selected when the other
stream failed). This didn't work; for one this check was missing from
one of the failure paths. And more importantly, both checked the
current_track array incorrectly.
Fix these issues, and move the failure handling code into a common
function.
CC: @mpv-player/stable
It possibly goes to sleep without actually starting to decode audio.
Possibly fixes a problem with --no-osc --no-video reported on IRC.
CC: @mpv-player/stable
Seems logical. For some reason, the player allows deselecting both audio
and video stream without quitting (a deliberate feature of which I have
no idea why it was added years ago), so this is needed.
Each subsystem (or similar thing) had an INITIALIZED_ flag assigned. The
main use of this was that you could pass a bitmask of these flags to
uninit_player(). Except in some situations where you wanted to
uninitialize nearly everything, this wasn't really useful. Moreover, it
was quite annoying that subsystems had most of the code in a specific
file, but the uninit code in loadfile.c (because that's where
uninit_player() was implemented).
Simplify all this. Remove the flags; e.g. instead of testing for the
INITIALIZED_AO flag, test whether mpctx->ao is set. Move uninit code
to separate functions, e.g. uninit_audio_out().
The messages "Audio: no audio" and "Video: no video" could be printed
twice each if initializing them failed. Prevent his silliness.
CC: @mpv-player/stable
Apparently this is what users want. When playing with normal speed,
nothing is done. When playing slower than normal, resampling is used
instead, because scaletempo (which does the pitch correction) adds
too many artifacts.
There's no real reason why audio_init_filter() should exist. Just use
af_init or af_reinit directly. (We lose a useless message; the same
information is printed in a quite close place with more details.)
Requires less code, and the way the filter chain is marked as having
failed to initialize allows just switching off audio instead of
crashing if trying to insert a volume filter in mixer.c fails, and
recreating the old filter chain fails too.
This would play some silence in case video was slower than audio. If
framedropping is already enabled, there's no other way to keep A/V
sync, short of changing audio playback speed (which would give worse
results). The --audiodrop option inserted silence if there was more
than 500ms desync.
This worked somewhat, but I think it was a silly idea after all. Whether
the playback experience is really bad or slightly worse doesn't really
matter. There also was a subtle bug with PTS handling, that apparently
caused A/V desync anyway at ridiculous playback speeds.
Just remove this feature; nobody is going to use it anyway.
Before this commit, there was AF_FORMAT_AC3 (the original spdif format,
used for AC3 and DTS core), and AF_FORMAT_IEC61937 (used for AC3, DTS
and DTS-HD), which was handled as some sort of superset for
AF_FORMAT_AC3. There also was AF_FORMAT_MPEG2, which used
IEC61937-framing, but still was handled as something "separate".
Technically, all of them are pretty similar, but may use different
bitrates. Since digital passthrough pretends to be PCM (just with
special headers that wrap digital packets), this is easily detectable by
the higher samplerate or higher number of channels, so I don't know why
you'd need a separate "class" of sample formats (AF_FORMAT_AC3 vs.
AF_FORMAT_IEC61937) to distinguish them. Actually, this whole thing is
just a mess.
Simplify this by handling all these formats the same way.
AF_FORMAT_IS_IEC61937() now returns 1 for all spdif formats (even MP3).
All AOs just accept all spdif formats now - whether that works or not is
not really clear (seems inconsistent due to earlier attempts to make
DTS-HD work). But on the other hand, enabling spdif requires manual user
interaction, so it doesn't matter much if initialization fails in
slightly less graceful ways if it can't work at all.
At a later point, we will support passthrough with ao_pulse. It seems
the PulseAudio API wants to know the codec type (or maybe not - feeding
it DTS while telling it it's AC3 works), add separate formats for each
codecs. While this reminds of the earlier chaos, it's stricter, and most
code just uses AF_FORMAT_IS_IEC61937().
Also, modify AF_FORMAT_TYPE_MASK (renamed from AF_FORMAT_POINT_MASK) to
include special formats, so that it always describes the fundamental
sample format type. This also ensures valid AF formats are never 0 (this
was probably broken in one of the earlier commits from today).
With e.g --start=-3 --audio-buffer=10 the decoder entered EOF state
before the initial sync was finished, entered STATUS_EOF, and just
started playing audio from a random position.
This doesn't handle seeking outside of the file, which is a different
case. E.g. --start=30:00 with audio and video enabled in a file shorter
than 30:00 will play a random last part of audio. This could perhaps be
fixed by using the hr-seek target for cutting audio, instead of the
video PTS, but that would be kind of intrusive, so don't do it for now.
The simpler solution, assuming audio EOF on video EOF, wouldn't work,
because we allow audio to start before video, or to last after video.
Somehow, there was a larger misunderstanding in the code: ao_buffer
does not need to be preserved over audio reinit for proper support of
gapless audio. The actual AO internal buffer takes care of this.
In fact, preserving ao_buffer just breaks audio resync. In the ordered
chapter case, end_pts is used, which means not all audio data in the
buffer is played, thus some data is left over when audio decoding
resumes on the next segment. This triggers some code that aborts resync
if there's "audio decoded" (ao_buffer contains something), but no PTS
is known (nothing was actually decoded yet).
Simplify, and always bind the output buffer to the decoder.
CC: @mpv-player/stable (maybe)
Probably no observable effect, but it's more correct. Setting audio to
EOF could have bad effects otherwise (anywhere the player logic for
example decides whether EOF was reached, and such).
Don't attempt to resync after speed changes. Note that most other cases
of audio reinit (like switching tracks etc.) still resync, but other
code paths take care of setting the audio_status accordingly.
This restores the old behavior of not trying to fix audio desync, which
was probably changed with commit 261506e3.
Note that the code as of now wasn't even entirely correct, since the A/V
sync values are slightly shifted. The dsync depends on the audio buffer
size, so a larger buffer size will show more extreme desync. Also see
mplayer2 commit 213a224e, which should fixed this - it was not merged
into mpv, because it disabled audio for too long, resulting in a worse
user experience. This is similar to the issue this commit attempts to
fix.
Fixes: #1042 (probably)
CC: @mpv-player-stable
This shouldn't change anything functionally.
Change the A/V desync message. --framedrop is enabled by default now, so
the text must be changed a little. I've never heard of audio outputs
messing up A/V sync recently, so remove that part.
Remove the unused ao_pts field.
Reorder 2 A/V sync related expressions so that they look the same.
In theory, timestamps can be negative, so we shouldn't just return -1
as special value.
Remove the separate code for clearing decode buffers; use the same code
that is used for normal seek reset.
