When a video-reconfig occurs with an image, the cache is not used
because find_cache_seek_range() checks if the start time was
initialized, but for images it stays at MP_NOPTS_VALUE. This makes
rotating large network images slow because they are re-downloaded on
every rotation.
Fix this by setting the timestamps of image cache ranges by altering a
condition.
cd59ea8afa removed an arbitrary start
offset added to the timer. However, it turns out that demux secretly
depends on this. When updating cache to actually read bytes from the
stream, there's a diff >= MP_TIME_S_TO_NS(1) check to make it only
update once every second. With the old MP_START_TIME macro, the initial
time value would always be at least 1e10, so this would also be true.
Since we don't have that offset anymore, now the initial time is less
than that so it is not updated and properties like file-size are 0. Just
be sure to always update if the last_speed_query is 0 (i.e. we just
started the player). Fixes#12869.
currently hysteresis-secs only works when the demuxer-max-bytes fills
up. but it's possible for the cache-secs/demuxer-readahead-secs to be
reached first.
in those cases, hysteresis-secs doesn't work and keeps buffering
non-stop. but the goal of this option was to save power by avoiding
non-stop buffering so go ahead and make it respect cache-secs as well.
additionally remove some redundant repetition from the docs.
since i was going to fix the include order of stdatomic, might as well
sort the surrouding includes in accordance with the project's coding
style.
some headers can sometime require specific include order. standard
library headers usually don't. but mpv might "hack into" the standard
headers (e.g pthreads) so that complicates things a bit more.
hopefully nothing breaks. if it does, the style guide is to blame.
replace it with <stdatomic.h> and replace the mp_atomic_* typedefs with
explicit _Atomic qualified types.
also add missing config.h includes on some files.
There's really no reason not to do this especially since sub-codepage
already defaults to auto. Also change logging in charset_conv since
telling us that the data is UTF-8 if the passed codepage value is "auto"
or "utf-8" is really not useful information (that's the expectation).
Previously if the demuxer didn't exist, then it could jump down and try
to free sinfo.filename before it was ever set thus segfaulting. Just
always set the struct unconditionally so we're always sure to free it.
Some demuxers actually close the stream right after they are finished
opening like cue. Since the stream->url is no longer copied with this
commit, that means it gets thrown away after the stream closes. This
leads to a use after free. We still need to allocate stream->url so fix
this another way.
This reverts commit 3e85df3b2d.
Yeah another try at this. So when inspecting lazy_stream_needs_wait, I
realized it had a curious !ds->reader_head condition. Actually, this is
what is messing everything up. This was originally added in
cf2b7a4997 for showing large negative sub
delay values correctly. It worked because the packet will eventually be
discarded during playback causing ds->reader_head not exist and thus the
next one will correctly be read ahead as needed.
But for the "switching subtitle tracks while paused" case, this is
actually bad. As the stream is read, eventually you'll find a packet and
set the reader_head. But it's not going to be the correct packet (unless
you're looking for the very first one), so you need to read more. This
won't happen because of the !ds->reader_head check and unlike the sub
delay case, nothing will eventually discard that packet since playback
isn't occuring. So read_packet exits earlier than it should and isn't
tried again, so the subtitle that you want won't show since the
returned packet has the wrong pts. All that needs to be done here is to
delete this one condition. There's already checks in place to make sure
that it's not read past the desired timestamp and for the sub delay case
(the only other time this logic is used), it makes no difference since
you won't read past the specified pts in the first place.
The stream selection state wasn't improved. I didn't realize this messed
with caches. All in all, just not a good idea. Back to drawing board I
guess.
This reverts commit f40bbfec4f.
This replaces the previous commit and makes more sense. The internal
demux marked tracks as eager depending on their type and for subtitles
it would always lazily read them unless there happened to be no
available av stream. However, we want the sub stream to be eager if the
player is paused. The existing subtitle is still preserved on the
screen, but if the user changes tracks that's when the problem occurs.
So to handle this case, propagate the mpctx->paused down to the stream
selection logic. This modifies both demuxer_refresh_track and
demuxer_select_track to take that boolean value. A few other parts of
the player use this, but we can just assume false there (no change in
behavior from before) since they should never be related to subtitles.
The core player code is aware of its own state naturally, and can always
pass the appropriate value so go ahead and do so. When we change the
pause state, a refresh seek is done on all existing subtitle tracks to
make sure their eager state is the appropriate value (i.e. so it's not
still set to eager after a pause and a track switch). Slightly invasive
change, but it works with the existing logic instead of going around it
so ultimately it should be a better approach. We can additionally remove
the old force boolean from sub_read_packets since it is no longer
needed.
Actually, I thought of a better way of handling this shortly after
merging this. Revert it and redo it in the next commit.
This reverts commit c2c157ebec.
a323dfae42 almost fixed subtitle tracks
disappearing when paused but it actually missed one part: the behavior
of demux_read_packet_async_until. It's a bit unintuitive, but for
subtitle streams, that function would only return the very first packet
regardless of whatever pts you pass to it. So the previous commit worked
on the very first subtitle, but not actually any of the others (oops).
This is because subtitle streams never marked as eager and thus never
actually read farther ahead. While the video is playing, this is OK, but
if we're paused and switching subtitle tracks then the stream should be
eagerly read. Luckily, the logic is already there in the function for
this. All we have to do add an extra argument to
demux_read_packet_async_until to force the stream to be read eagerly and
then it just works. Be sure to unset the eager flag when we're done.
Actually fixes the bug for real this time.
Several parts of the code need to access options here. There's no point
in hiding it demux.c so just expose it in the demux.h header. This means
pulling it out of demux_internal and putting it in the demuxer struct
instead.
A bit different from the OPT_REPLACED/OPT_REMOVED ones in that the
options still possibly do something but they have a deprecation
message. Most of these are old and have no real usage. The only
potentially controversial ones are the removal of --oaffset and
--ovoffset which were deprecated years ago and seemingly have no real
replacement. There's a cryptic message about --audio-delay but who
knows. The less encoding mode code we have, the better so just chuck
it.
c784820454 introduced a bool option type
as a replacement for the flag type, but didn't actually transition and
remove the flag type because it would have been too much mundane work.
In debug mode the macro causes an assertion failure.
In release mode it works differently and tells the compiler that it can
assume the codepath will never execute. For this reason I was conversative
in replacing it, e.g. in mpv-internal code that exhausts all valid values
of an enum or when a condition is clear from directly preceding code.
Without this change the same track encoded as Opus - which requires R128
tagging - and e.g. Vorbis with ReplayGain tagging have different volumes.
This is caused by ReplayGain 2 having a higher reference level of -18 dB
LUFS, while EBU R128 has a lower reference level of -23 dB LUFS.
For the results of gain application to match, the read EBU R128
values need to be boosted according to the difference in reference
levels.
Patch inspired by mpd's source code.
Buffering ahead nonstop into the cache results in nonstop disk or network
activity to read stream data from wherever it may originate. Currently,
there's no way to configure the demuxer to back off once it's buffered
ahead enough data, since the cache limit will be perpetually not-reached as
a stream continues to play, until the entire stream is eventually buffered.
On a laptop with an i9-12900H with decoding performed by the iGPU,
watching a locally-saved 1080p video which hasn't been buffered into the
page cache consumes approximately 15 W even with caching enabled. When
configuring a hysteresis to make the demuxer back off, power consumption
drops to 9 W when watching the same video, resulting in a whopping 6 W of
power savings.
To make it possible to attain significant power savings via caching, add
a --demuxer-hysteresis-secs option to configure a hysteresis to make the
demuxer back off until there's only the configured number of seconds
remaining in the cache from the current playback position.
This feature is disabled by default.
This seems to work on gcc, clang and mingw as-is, but I made it
conditional on __GNUC__ just in case, even though I can't figure out
which compilers we care about that don't export this define.
Also replace all instances of assert(0) in the code by MP_UNREACHABLE(),
which is a strict improvement.
in->byte_level_seeks field is written and modified inside
update_bytes_read at the same time when demux_get_reader_state
is executing, which locks the demux thread mutex. This results
in a data race, reported by Thread Sanitizer when playing mp3 file
of sufficient long length.
Though, only when the output format is matroska, to avoid muxing errors.
This is quite useful when the input has ASS subtitles, as they tend to
rely on embedded fonts.
dump_cache() calls qsort() to order an array of pointers, while the
comparator forgets it's receiving pointers to pointers.
Since cache-dumping over multiple cache ranges is fairly rare, this
seems to have gone unnoticed.
Unfortunately, attached pictures (from tags etc.) are treated as video
tracks. That meant --sub-create-cc-track added a CC track for them as
well. Stop doing that.
See: #7608
Add an infrastructure for collecting performance-related data, use it in
some places. Add rendering of them to stats.lua.
There were two main goals: minimal impact on the normal code and normal
playback. So all these stats_* function calls either happen only during
initialization, or return immediately if no stats collection is going
on. That's why it does this lazily adding of stats entries etc. (a first
iteration made each stats entry an API thing, instead of just a single
stats_ctx, but I thought that was getting too intrusive in the "normal"
code, even if everything gets worse inside of stats.c).
You could get most of this information from various profilers (including
the extremely primitive --dump-stats thing in mpv), but this makes it
easier to see the most important information at once (at least in
theory), partially because we know best about the context of various
things.
Not very happy with this. It's all pretty primitive and dumb. At this
point I just wanted to get over with it, without necessarily having to
revisit it later, but with having my stupid statistics.
Somehow the code feels terrible. There are a lot of meh decisions in
there that could be better or worse (but mostly could be better), and it
just sucks but it's also trivial and uninteresting and does the job. I
guess I hate programming. It's so tedious and the result is always shit.
Anyway, enjoy.
Florian Albrechtskirchinger