Helper for the ab-loop-dump-cache command, see manpage additions.
This is kind of shit. Not only is this a very "special" feature, but it
also vomits more messy code into the big and already bloated demux.c,
and the implementation is sort of duplicated with the dump-cache code.
(Except it's different.) In addition, the results sort of depend what a
video player would do with the dump-cache output, or what the user wants
(for example, a user might be more interested in the range of output
audio, instead of the video).
But hey, I don't actually need to justify it. I'm only justifying it for
fun.
That's right, and it's probably not the end of it. I'll just claim that
I have no idea how to create a proper user interface for this, so I'm
creating multiple partially-orthogonal, of which some may work better in
each of its special use cases.
Until now, there was --record-file. You get relatively good control
about what is muxed, and it can use the cache. But it sucks that it's
bound to playback. If you pause while it's set, muxing stops. If you
seek while it's set, the output will be sort-of trashed, and that's by
design.
Then --stream-record was added. This is a bit better (especially for
live streams), but you can't really control well when muxing stops or
ends. In particular, it can't use the cache (it just dumps whatever the
underlying demuxer returns).
Today, the idea is that the user should just be able to select a time
range to dump to a file, and it should not affected by the user seeking
around in the cache. In addition, the stream may still be running, so
there's some need to continue dumping, even if it's redundant to
--stream-record.
One notable thing is that it uses the async command shit. Not sure
whether this is a good idea. Maybe not, but whatever. Also, a user can
always use the "async" prefix to pretend it doesn't.
Much of this was barely tested (especially the reinterleaving crap),
let's just hope it mostly works. I'm sure you can tolerate the one or
other crash?
It makes some slight sense and helps with one of the following commits.
Also rename that other function to make it sound less similar to
find_seek_target().
Always set max_bytes_bw to 0 if seekable cache is disabled, instead at
the place of its use. This is the only use of it, so the commit should
not change any behavior.
(Alternatively, this could drop the max_bytes_bw variable, use the
option directly, and keep the old code that resets it on use of the
cache is disabled.)
Until now, the following could happen: if you set a 1GB forward cache,
and a 1GB backward cache, and you opened a 2GB file, it would prune away
the data cached at the start as playback progressed past the 50% mark.
With this commit, nothing gets pruned, because the total memory usage
will still be 2GB, which equals the total allowed memory usage of 1GB +
1GB.
There are no explicit buffers (every packet is malloc'ed and put into a
linked list), so it all comes down to buffer size computations. Both
reader and prune code use these sizes to decide whether a new packet
should be read / an old packet discarded. So just add the remaining free
"space" from the forward buffer to the available backward buffer. Still
respect if the back buffer is set to 0 (e.g. unseekable cache where it
doesn't make sense to keep old packets).
We need to make sure that the forward buffer can always append, as long
as the forward buffer doesn't exceed the set size, even if the back
buffer "borrows" free space from it. For this reason, always keep 1 byte
free, which is enough to allow it to read a new packet. Also, it's now
necessary to call pruning when adding a packet, to get back "borrowed"
space that may need to be free'd up after a packet has been added.
I refrained from doing the same for forward caching (making forward
cache use unused backward cache). This would work, but has a
disadvantage. Assume playback starts paused. Demuxing will stop once the
total allowed low total cache size is reached. When unpausing, the
forward buffer will slowly move to the back buffer. That alone will not
change the total buffer size, so demuxing remains stopped. Playback
would need to pass over data of the size of the back buffer until
demuxing resume; consider this unacceptable. Live playback would break
(or rather, would not resume in unintuitive ways), even normal streaming
may break if the server invalidates the URL due to inactivity. As an
alternative implementation, you could prune the back buffer immediately,
so the forward buffer can grow, but then the back buffer would never
grow. Also makes no sense.
As far as the user interface is concerned, the idea is that the limits
on their own aren't really meaningful, the purpose is merely to vaguely
restrict the cache memory usage. There could be just a single option to
set the total allowed memory usage, but the separate backward cache
controls the default ratio of backward/forward cache sizes. From that
perspective, it doesn't matter if the backward cache uses more of the
total buffer than assigned, if the forward buffer is complete.
The last_eof field is the last known EOF state from the underlying
demuxer. Normally, seeks reset it, because obviously if seek back into
the middle of the file, you don't want last_eof to have a "wrong" value
for a short time window (until a packet is read, which would reset the
field to its correct value).
This shouldn't happen during cache seeks, because you don't touch the
underlying demuxer state.
At first, I made this change because some other work in progress
required it. It turned out that it was unnecessary, but keep the change
anyway, since it's still correct and makes the logic cleaner.
Make most of the demuxer options runtime-changeable. This includes the
cache options and stream recording. The manpage documents some of the
possibly weird issues related to this.
In particular, the disk cache isn't shuffled around if the setting
changes at runtime.
Although this is not useful in general, it makes --stream-record work
with a certain video streaming service by a large dystopian company.
In the general case, this fails because normal muxing can, quite
obviously, not handle the segmented metadata in the packets. (There
isn't even a file format which could handle these, except possibly mp4.)
On the other hand, ytdl merely uses timeline/EDL to emulate DASH
streaming (unfortunately), which does not use the segmented stuff, and
stream recording will actually work.
Retarded webshit streaming protocols (well, DASH) chop a stream into
small fragments, and move unchanging header parts to an "init" fragment
to save some bytes (in the case at hand about 300 bytes for each
fragment that is 100KB-200KB, sure was worth it, fucking idiots).
Since mpv uses an even more retarded hack to inefficiently emulate DASH
through EDL, it opens a new demuxer for every fragment. Thus the
fragment needs to be virtually concatenated with the init fragment. (To
be fair, I'm not sure whether the alternative, reusing the demuxer and
letting it see a stream of byte-wise concatenated fragmenmts, would
actually be saner.)
demux_lavc.c contained a hack for this. Unfortunately, a certain shitty
streaming site by an evil company, that will bestow dytopia upon us soon
enough, sometimes serves webm based DASH instead of the expected mp4
DASH. And for some reason, libavformat's mkv demuxer can't handle the
init fragment or rejects it for some reason. Since I'd rather eat
mushrooms grown in Chernobyl than debugging, hacking, or (god no)
contributing to FFmpeg, and since Chernobyl is so far away, make it work
with our builtin mkv demuxer instead.
This is not hard. We just need to copy the hack in demux_lavf.c to
demux_mkv.c. Since I'm not _that_ much of a dumbfuck to actually do
this, remove the shitty gross demux_lavf.c hack, and replace it by a
slightly less bad generic implementation (stream_concat.c from the
previous commit), and use it on all demuxers. Although this requires
much more code, this frees demux_lavf.c from a hack, and doesn't require
adding a duplicated one to demux_mkv.c, so to the naive eye this seems
to be a much better outcome.
Regarding the code, for some reason stream_memory_open() is never meant
to fail, while stream_concat_open() can in extremely obscure situations,
and (currently) not in this case, but we handle failure of it anyway.
Yep.
Timeline (demux_timeline, for EDL and mkv ordered chapters) are a mess,
because it's the only nested demuxer case. Part of the mess comes from
shared struct stream pointers. This makes no sense, because the wrapper
(demux_timeline) doesn't have any business setting it.
Try to lessen it by not passing down streams. Instead, pass down NULL.
This prevents unintended interference, and tightens the ownership rules.
Now a demuxer always owns its stream.
On the other hand, demuxer->stream can now be NULL. This was never the
case before, and consequently there will be new bugs. At least they will
be spotted, because they've been bugs before.
struct stream is also used to access stream properties (such as whether
something is considered a network stream). Most of these have been
mirrored in struct demuxer (because the frontend has been forbidden to
access struct stream because of threading). But during initialization
was still used, so introduce an awkward struct parent_stream_info, which
unifies these.
Commit e0419fb181 changed demux_is_network_cached() to use
demuxer->stream->streaming instead of demuxer->is_network. To enable
timeline stuff to use the cache anyway, change it so that both flags can
contribute to it. The stream NULL-check is obviously due to changes in
this commit.
Instead of having to rely on the protocol matching, make a function that
creates a stream from a stream_info_t directly. Instead of going through
a weird indirection with STREAM_CTRL, add a direct argument for non-text
arguments to the open callback. Instead of creating a weird dummy
mpv_global, just pass an existing one from all callers. (The latter one
is just an artifact from the past, where mpv_global wasn't available
everywhere.)
Actually I just wanted a function that creates a stream without any of
that bullshit. This goal was slightly missed, since you still need this
heavy "constructor" just to setup a shitty struct with some shitty
callbacks.
The previous commit broke audio playback (maybe this is what 4. was
about?). But it wasn't the fault of the commit; it just exposed
pre-existing issues.
If the packet queue search can't get all packets, it checked
queue->is_bof to see whether there could be previous packets. But
obviously, is_bof can be set, even if the search start packet wasn't the
first one.
This was especially observable with audio, because audio by default
needs preroll packets, and plays artifacts if they're missing.
Fix by using the BOF playback condition for this purpose too.
In theory, backward demuxing does way too much work by doing a full
cache seek every time you need to step back through a packet queue. In
theory, it would be exceedingly more efficient to just iterate backwards
through the queue, but this is not possible because I'm too stingy to
add 8 bytes per packet for backlinks. (In theory, you could probably
come up with some sort of deque, that'd allow efficient iteration into
any direction, but other requirements make this tricky, and I'm
currently too dumb/lazy to do this. For example, the queue can grow to
millions of elements, all while packet pointers need to stay valid.)
Another possibility is to "locally" seek the queue. This still has less
overhead than a full seek.
Also, it just so happens that, as a side effect, this avoids performing
range merging, which commit f4b0e7e942 "broke". That wasn't a bug at
all, but since range joining is relatively slow, avoiding it is good.
This is really just a coincidental side effect, I'm not even quite sure
why it happens this way.
There are 4 ugly things about this change:
1. To get a keyframe "before" a certain one, we recompute the target
PTS, and then subtract 0.001 as arbitrary number to "fudge" it. This
isn't the first place where this is done, and hey, it wasn't my damn
idea that MPlayer should use floats for timestamps. (At first, it even
used 32 bit timestamps.)
2. This is the first time reader_head is reset to an earlier position
outside of the seek code. This might cause conceptual problems since
this code is now "duplicated".
3. In theory, find_backward_restart_pos() needs to be run again after
the backstep. Normally, the seek code calls it explicitly. We could call
it right in the new code, but then the damn function would be recursive.
We could shuffle the code around to make it a loop, but even then
there'd be an offchance into running into an unexpected corner case (aka
subtle bug), where it would loop forever. To avoid refactoring the code
and having to think too hard about it, make it deferred - add some new
state and the check_backward_seek() function for this. Great, even more
subtle mutable state for this backwards shit.
4. I forgot this one, but I can assure you, it's bad.
Without doubt someone will have to clean up this slightly in the future
(or rip it out), but it won't be me.
This code tries to determine the "current" position, which is used as
base for the seek target when it needs to seek back more (the point is
to prevent seeking back too far). But compute_keyframe_times() almost
computes the same thing, so use that. Unfortunately needs a forward
declaration.
("Almost", because it differs in some details that should not really
matter.)
demux_packet.next should not be used outside of demux.c, and in this
case it's a packet that was just passed to demux.c from the outside.
demux_packet.stream is already set by the demuxer, and this is assured
by the add_packet_locked() caller.
We don't care much about this case, because backward playback can fail
terribly without a good way to detect it, so this was fine.
However, this froze in certain situations. Reading from a subtitle file
for which backward demuxing failed could make it get stuck in
demux_read_packet_async() in unthreaded mode. (That we don't support
backwards subtitle decoding anyway doesn't matter for this.)
So aggressively disable backward demuxing to prevent worse in these
situations. The behavior will still be awful, because the frontend is
still in backwards playback mode, but at least it won't freeze.
Somewhat similar to the old --cache-file, except for the demuxer cache.
Instead of keeping packet data in memory, it's written to disk and read
back when needed.
The idea is to reduce main memory usage, while allowing fast seeking in
large cached network streams (especially live streams). Keeping the
packet metadata on disk would be rather hard (would use mmap or so, or
rewrite the entire demux.c packet queue handling), and since it's
relatively small, just keep it in memory.
Also for simplicity, the disk cache is append-only. If you're watching
really long livestreams, and need pruning, you're probably out of luck.
This still could be improved by trying to free unused blocks with
fallocate(), but since we're writing multiple streams in an interleaved
manner, this is slightly hard.
Some rather gross ugliness in packet.h: we want to store the file
position of the cached data somewhere, but on 32 bit architectures, we
don't have any usable 64 bit members for this, just the buf/len fields,
which add up to 64 bit - so the shitty union aliases this memory.
Error paths untested. Side data (the complicated part of trying to
serialize ffmpeg packets) untested.
Stream recording had to be adjusted. Some minor details change due to
this, but probably nothing important.
The change in attempt_range_joining() is because packets in cache
have no valid len field. It was a useful check (heuristically
finding broken cases), but not a necessary one.
Various other approaches were tried. It would be interesting to list
them and to mention the pros and cons, but I don't feel like it.
Backwards demuxing usually seeks back back by a "random" amount (set by
a user option) when it needs new preceding packets. It turns out a past
change made these backwards seek amounts add up when it didn't need to
(i.e. subtracting the amount from the seek pos without properly
resetting it), which could possibly slow down playback as it went on.
The reason for this was that back_seek_pos was set for every stream on
every seek. This made the reset not affect other streams (in particular
streams which weren't used and never were reset, or which didn't reset
that often). But as the commit adding it showed, this is needed only to
set the initial position. So do that.
Fixes: "demux: fix initial backward demuxing state in some cases"
When packet appending sets the start of the range, it adjusts the range
by seek_preroll. Do this when packets are pruned from the start of the
range too.
(Yeah, seek_preroll handling is probably broken in some other cases. It
was halfhearted to begin with.)
The main thing this commit does is removing demux_packet.kf_seek_pts. It
gets rid of 8 bytes per packet. Which doesn't matter, but whatever.
This field was involved with much of seek range updating and pruning,
because it tracked the canonical seek PTS (i.e. start PTS) of a packet
range. We have to deal with timestamp reordering, and assume the start
PTS is the lowest PTS across all packets (not necessarily just the first
packet). So knowing this PTS requires looping over all packets of a
range (no, the demuxer isn't going to tell us, that would be too sane).
Having this as packet field was perfectly fine. I'm just removing it
because I started hating extra packet fields recently.
Before this commit, this value was cached in the kf_seek_pts field (and
computed "incrementally" when adding packets). This commit computes the
value on demand (compute_keyframe_times()) by iterating over the placket
list. There is some similarity with the state before 10d0963d85,
where I introduced the kf_seek_pts field - maybe I'm just moving in
circles. The commit message claims something about quadratic complexity,
but if the code before that had this problem, this new commit doesn't
reintroduce it, at least. (See below.)
The pruning logic is simplified (I think?) - there is no "incremental"
cached pruning decision anymore (next_prune_target is removed), and
instead it simply prunes until the next keyframe like it's supposed to.
I think this incremental stuff was only there because of very old code
that got refactored away before. I don't even know what I was thinking
there, it just seems complex. Now the seek range is updated when a
keyframe packet is removed.
Instead of using the kf_seek_pts field, queue->seek_start is used to
determine the stream with the lowest timestamp, which should be pruned
first. This is different, but should work well. Doing the same as the
previous code would require compute_keyframe_times(), which would
introduce quadratic complexity.
On the other hand, it's fine to call compute_keyframe_times() when the
seek range is recomputed on pruning, because this is called only once
per removed keyframe packet. Effectively, this will iterate over the
packet list twice instead of once, and with some locality. The same
happens when packets are appended - it loops over the recently added
packets once again. (And not more often, which would go above linear
complexity.)
This introduces some "cleverness" with avoiding calling
update_seek_ranges() even when keyframe packets added/removed, which is
not really tightly coupled to the new code, and could have been in a
separate commit.
Removing next_prune_target achieves the same as commit b275232141,
which is hereby reverted (stale is_bof flags prevent seeking before the
current range, even if the beginning of the file was pruned). The seek
range is now strictly computed after at least one packet was removed,
and stale state should not be possible anymore.
Range joining may over-allocate the index a little. It tried hard to
avoid this before by explicitly freeing the old index before creating a
new one. Now it iterates over the old index while adding the entries to
the new one, which is simpler, but may allocate twice the memory in the
worst case. It's not going to matter for anything, though.
Seeking will be slightly slower. It needs to compute the seek PTS values
across all packets in the vicinity of the seek target. The previous code
also iterated over these packets, but now it iterates one packet range
more.
Another minor detail is that the special seeking code for SEEK_FORWARD
goes away. The seeking code will now iterate over the very last packet
range too, even if it's incomplete (i.e. packets are still being
appended to it). It's fine that it touches the incomplete range, because
the seek_end fields prevent that anything particularly incorrect can
happen. On the other hand, SEEK_FORWARD can now consider this as seek
target, which the deleted code had to do explicitly, as kf_seek_pts was
unset for incomplete packet ranges.
Obviously doesn't sense with this order. The git history shows that this
comment was touched multiple times, without ever fixing it. It was
originally added in 2016, where the "for" was missing. Later, the "for"
was added, but to the wrong position.
What the fuck?
The old implementation didn't work for the OGG case. Discard the old
shit code (instead of fixing it), and write new shit code. The old code
was already over a year old, so it's about time to rewrite it for no
reason anyway.
While it's true that the old code appears to be broken, the main reason
to rewrite this is to make it simpler. While the amount of code seems to
be about the same, both the concept and the actual tag handling are
simpler. The result is probably a bit more correct.
The packet struct shrinks by 8 byte. That fact that it wasted 8 bytes
per packet for a rather obscure use case was the reason I started this
at all (and when I found that OGG updates didn't work). While these 8
bytes aren't going to hurt, the packet struct was getting too bloated.
If you buffer a lot of data, these extra fields will add up. Still quite
some effort for 8 bytes. Fortunately, it's not like there are any
managers that need to be convinced whether it's worth doing. The freedom
to waste time on dumb shit.
The old implementation attached the current metadata to each packet.
When the decoder read the packet, the packet's metadata was made
current. The new implementation stores metadata as separate list, and
requires that the player frontend tells it the current playback time,
which will be used to find the currently valid metadata. In both cases,
the objective was to correctly update metadata even if a lot of data is
buffered ahead (and to update them correctly when seeking within the
demuxer cache).
The new implementation is actually slightly more correct, because it
uses the playback time for the metadata lookup. Consider if you have an
audio filter which buffers 15 seconds (unfortunately such a filter
exists), then the old code would update the current title 15 seconds too
early, while the new one does it correctly.
The new code also simplifies mixing the 3 metadata sources (global, per
stream, ICY). We assume these aren't mixed in a meaningful way. The old
code tried to be a bit more "exact". I didn't bother to look how the old
code did this, but the new code simply always "merges" with the previous
metadata, so if a newer tag removes a field, it's going to stick around
anyway.
I tried to keep it simple. Other approaches include making metadata a
special sh_stream with metadata packets. This would have been
conceptually clean, but the implementation would probably have been
unnatural (and doesn't match well with libavformat's API anyway). It
would have been nice to make the metadata updates chapter points (makes
a lot of sense for the intended use case, web radio current song
information), but I don't think it would have been a good idea to make
chapters suddenly so dynamic. (Still an idea to keep in mind; the new
code actually makes it easier to work towards this.)
You could mention how subtitles are timed metadata, and actually are
implemented as sparse packet streams in some formats. mp4 implements
chapters as special subtitle stream, AFAIK. (Ironically, this is very
not-ideal for files. It would be useful for streaming like web radio,
but mp4 is extremely bad for streaming by design for other reasons.)
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla
Remove the duplicated creation of the first range. Explicitly destroy
ranges, including the last one on final deinit.
It looks like this also fixes a leak of removed range structs, which was
never noticed because they're so small, and were freed on final deinit
due to having the demuxer as talloc parent.
This improves upon the previous commit too (that change should have
been part of it I guess). Sub-demuxers (demux_timeline only) now
automatically don't use the cache (like it was intended by the previous
commit). The cache is "initialized" (or disabled) last in the recursive
call chain, which is messy, but this sub demuxer stuff FUCKING SUCKS, as
mentioned in the previous commit message. This would be no problem if
the caching layer and actual demuxer implementations were separate.
Most of this change has no purpose. Might make (de-)initialization of
further cache exerpiments simpler.
It seems the so called demuxer cache wasn't really disabled for
sub-demuxers (timeline stuff). This was relatively harmless, since the
actual packet data was shared anyway via refcounting. But with the
addition of a mmap cache backend, this may change a lot.
So strictly disable any caching for sub-demuxers. This assumes that
users of sub-demuxers (only demux_timeline.c by now?) strictly use
demux_read_any_packet(), since demux_read_packet_async() will require
some minor read-ahead if a low level packet read returned a packet for a
different stream.
This requires some awkward messing with this fucking heap of trash. The
thing that is really wrong here is that the demuxer API mixes different
concepts, and sub-demuxers get the same API as decoders, and use the
cache code.
The demuxer cache tries to track the number of bytes allocated for the
cache. In addition to the packet queue, the seek index is another data
structure that roughly depends on the amount of packets cached. So the
index size should somehow be part of the total number of bytes tracking.
Until now, this was handled with KF_SEEK_ENTRY_WORST_CASE, basically a
shitty heuristic. It was a guess (and probably rather an upper bound
than a lower bound). The implementation details made it annoying, and it
was conceptually inaccurate too.
Change this, and instead simply add the index size to the total cache
size. This essentially makes it part of the backbuffer. It's nice that
this cleanly decouples it from the packet size tracking itself.
Since it's part of the backbuffer number of bytes now, packet pruning
can't necessarily free enough space in the backbuffer anymore. Before
this commit, the backbuffer consisted of packets only, so it was
possible to reduce its size to 0 by pruning all packets until the
decoder reader position, at which point a packet was accounted as
forward buffered. Now the index is added to this, and it can't be
pruned. Replace the assert() because of this changed invariant.
Someone who rams a knife into his own hand just to see what happens is
normally put in a psychiatric ward. But in software, this is acceptable
behavior. Programs are not supposed to crash just because a user did
something unreasonably dumb.
Switching tracks during backward playback is such a thing. It triggered
an assertion because the newly enabled stream was not properly
initialized for backward playback. Fix this, and make it actually work
(mostly; it still takes a "while" until playback recovers fully).
This actually makes some aspects of initialization slightly cleaner.
Not sure if this is bug-free. You _always_ make bugs when writing a
binary search from scratch (and such is the curse of C, though if I did
this in C++ it would probably end in blood). It seems to work though,
checking against the normal linear search.
It's slightly faster. Not much.
I wonder if the termination condition can be written in a nicer/elegant
way. I guess the fact that it's not a == predicate makes this slightly
messier?
The purpose of the seek index is to avoid having to iterate over the
full linked list of cached packets, which should speed up seeking. Until
now, there was an excuse of a seek index, that didn't really work.
The idea of the old index was nice: have a fixed number of entries (no
need to worry about exceeding memory requirements), which are
"stretched" out as the cache gets bigger. The size of it was 16 entries,
which in theory should speed up seeking by the factor 16, given evenly
spaced out entries. To achieve this even spacing, it attempted to "thin
out" the index by half once the index was full (see e.g. index_distance
field). In my observations this didn't really work, and the distribution
of the index entries was very uneven. Effectively, this did nothing. It
probably worked once and I can't be assed to debug my own shit code.
Writing new shit code is more fun.
Write new shit code for fun. This time it's a complete index. It's kept
in a ringbuffer (for easier LIFO style appending/removing), which is
resized with realloc if it becomes too small.
Actually, the index is not completely completely; it's still "thinned
out" by a hardcoded time value (INDEX_STEP_SIZE). This should help with
things like audio or crazy subtitle tracks (do they still create
those?), where we can just iterate over a small part of the linked
packet list to get the exact seek position. For example, for AAC audio
tracks with typical samplerates/framesizes we'd iterate about 50 packets
in the linked list.
The results are good. Seeking in large caches is much faster now,
apparently at least 1 or 2 orders of magnitude. Part of this is because
we don't need to touch every damn packet in a huge linked list (bad
cache behavior - the index is a linear memory region instead), but
"thinning" out the search space also helps. Both aspects can be easily
tested (setting INDEX_STEP_SIZE to 0, and replacing e->pts with
e->pkt->kf_seek_pts in find_seek_target()).
This does use more memory of course. In theory, we could tolerate memory
allocation failures (the index is optional and only for performance),
but I didn't bother and inserted an apologetic comment instead, have fun
with the shit code). the memory usage doesn't seem to be that bad,
though. Due to INDEX_STEP_SIZE it's bounded by the file duration too.
Try to account for the additional memory usage with an approximation
(see KF_SEEK_ENTRY_WORST_CASE). It's still a bit different, because the
index needs a single, potentially large allocation.
The search was slightly more complicated and slow than it had to be. It
didn't assume that the packet list was sorted, which is responsible for
much of this. (I think the search code was borrowed from demux_mkv.c,
which does not sort index entries.)
There was a half-hearted attempt to make it exit early, but it was
mostly ineffective.
Simplify the code based on the assumption that the list is sorted. This
will exit the search loop once the worst case candidate entry was
checked.
Mostly about the packet queue and the subtitle handling of it.
(This mess sure sounds like a good argument to give up the separate
stream queues, and using a single packet queue per cached range.)
Until now, this usually passed a single audio frame to the decoder, and
then did a backstep operation (cache seek + frame search) again. This is
probably not very efficient, especially considering it has to search the
packet queue from the "start" every time again.
Also, with most audio codecs, an additional "preroll" frame was passed
first. In these cases, the preroll frame would make up 50% of audio
decoding time. Also not very efficient.
Attempt to fix this by returning multiple frames at once. This reduces
the number of backstep operations and the ratio the preoll frames. In
theory, this should help efficiency. I didn't test it though, why would
I do this? It's just a pain. Set it to unscientific 10 frames.
(Actually, these are 10 keyframes, so it's much more for codecs like
TrueHD. But I don't care about TrueHD.)
This commit changes some other implementation details. Since we can
return more than 1 non-preroll keyframe to the decoder, some new state
is needed to remember how much. The resume packet search is adjusted to
find N ("total") keyframe packets in general, not just preroll frames.
I'm removing the special case for 1 preroll packet; audio used this, but
doesn't anymore, and it's premature optimization anyway.
Expose the new mechanism with 2 new options. They're almost completely
pointless, since nobody will try them, and if they do, they won't
understand what these options truly do. And if they actually do, they
most likely would be capable of editing the source code, and we could
just hardcode the parameters. Just so you know that I know that the
added options are pointless.
The following two things are truly unrelated to this commit, and more
like general refactoring, but fortunately nobody can stop me.
Don't set back_seek_pos in dequeue_packet() anymore. This was sort of
pointless, since it was set in find_backward_restart_pos() anyway (using
some of the same packets). The latter function tries to restrict this to
the first keyframe range though, which is an optimization that in theory
might break with broken files (duh), but in these cases a lot of other
things would be broken anyway.
Don't set back_restart_* in dequeue_packet(). I think this is an
artifact of the old restart code (cf. ad9e473c55). It can be done
directly in find_backward_restart_pos() now. Although this adds another
shitty packet search loop, I prefer this, because clearer what's
actually happening.
The size of all forward buffered packets is used to control maximum
buffering.
Until now, this size was incrementally adjusted, but had to be
recomputed on seeks within the cache. Doing this was actually pretty
expensive. It iterates over a linked list of separate memory allocations
(which are probably spread all over the heap due to the allocation
behavior), and the demux_packet_estimate_total_size() call touches a lot
of further memory locations. I guess this affects the cache rather
negatively. In an unscientific test, the recompute_buffers() function
(which contained this loop) was responsible for roughly half of the time
seeking took.
Replace this with a way that computes the buffered size between 2
packets in constant times. The demux_packet.cum_pos field contains the
summed sizes of all previous packets, so subtracting cum_pos between two
packets yields the size of all packets in between. We can do this
because we never remove packets from the middle of the queue. We only
add packets to the end, or remove packets at the beginning.
The tail_cum_pos field is needed because we don't store the end position
of a packet, so the last packet's position would be unknown. We could
recompute the "estimated" packet size, or store the estimated size in
the packet struct, but I just didn't like this.
This also removes the cached fw_bytes fields. It's slightly nicer to
just recompute them when needed. Maintaining them incrementally was
annoying. total_size stays though, since recomputing it isn't that cheap
(would need to loop over all ranges every time).
I'm always using uint64_t for sizes. This is certainly needed (a stream
could easily burn through more than 4GB of data, even if much less of
that is cached). The actual cached amount should always fit into size_t,
so it's casted to size_t for printfs (yes, I hate the way you specify
stdint.h types in printfs, the less I have to use that crap, the
better).
In ancient times, the number of packets was used to limit excessive
read-ahead. This was completely replaced by tracking the size in bytes.
The number of packets was used in debugging output only.
In one case (packet got demuxed and is added to a queue), only log
whether there were packets on this stream before. (Unknown whether it's
useful.)
In another case (queue overflow), actually count the number of packets.
It's vaguely useful, and the message with the number of packets is shown
only once after a seek reset, so it doesn't matter whether it's slow.
Some files don't start with keyframe packets. Normally, this is not
sane, but the sample file which triggered this was a cut TV capture
transport stream. And this shouldn't happen anyway.
Introduce a further heuristic: if the last seek target was before the
start of the cached data, and the start of the cache is marked as BOF
(beginning of file), then we won't find anything better. This is
possibly a bit shaky, because both seek_start and back_seek_pos weren't
made for this purpose. But I can't come up with situations where this
would actually break. (Leave this to shitty broken files I hit later.)
I also considered finding the first packet in the cache that is marked
as keyframe, i.e. the first actual seek target, and comparing it to
"first", but I didn't like it much. Well whatever.
It's a bit silly that this caused a hard freeze (and similar issues
still will). The problem is that the demuxer holds the lock and has no
reason to release it. And in general, there's a single lock for the
entire demuxer cache. Finer grained locking would probably not make much
sense. In theory status of available data and maybe certain commands to
the demuxer could be moved to separate locks, but it would raise
complexity, and you'd probably still need to get the central lock in
some cases, which would deadlock you anyway.
It would still be nice if some minor corner case in the wonderfully
terrible and complex backward demuxer state machine couldn't lock up the
player. As a hack, unlock and then immediately lock again. Depending on
the OS mutex implementation, this may give other waiters a chance to
grab the lock. This is not a guarantee (some OSes may for example not
wake up other waiters until the next time slice or something), but works
well on Linux.
The step_backwards function set reader_head to the start of the current
cache range. This was completely unnecessary and made it _much_ slower.
Remove the code that adjusts reader_head. Merge the rest of the code
into the only caller and remove the function.
The comment on the removed code was quite right. It was "inefficient".
Removing it delegates going to an early position to the normal seek
code, triggered by find_backward_restart_pos() incremental back seek
logic. I suppose especially audio benefits from this, because this
happens for every single audio packet (except maybe freaky bullshit like
TrueHD, which has "keyframes").
The blabla about performance in the removed comments is still true, but
now applies to the seek code itself only.
Fixes "mpv file.mkv --cache --demuxer-cache-wait --play-dir=backward",
and other situations where the demuxer cache contains the entire file,
and playback is to start from the end. It also can be triggered when
starting playback normally with --cache, and once everything is in the
cache, enabling backward playback and seeking past EOF.
In all cases, the cache seek will set reader_head=NULL (because you
seeked to/past EOF). Then the code (the one modified by this commit)
sees that ds->queue->is_bof==true, and thinks we've reached BOF
(beginning of file) while searching for a useful packet, i.e. we found
nothing and playback really can only end.
Obviously this is nonsense, we've found only nothing if we actually
searched from the beginning, not some "random" reader_head (== first)
value that does not include the entire cache. That means the condition
should trigger only if the start of the search (first variable) points
to the beginning of the cache (ds->queue->head).
Not taking this if means we'll seek to an earlier position and retry.
Also, a seek before the beginning of the cache will always end up with
reader_head==ds->queue->head, i.e. we'll terminate properly.
That comment was quite right.
Together with the previous commit, this seems to make backward playback
work in files with vorbis and mp3 audio codecs.
For Vorbis (with libavcodec's decoder, didn't test libvorbis), the first
packet was just always completely discarded. This happened even though
we tell libavcodec that we do discarding of padding manually. It simply
happened inside the codec, not libavcodec's general initial padding
handling. In addition, the first output decoded frame seems to contain
partial data. (Unlike the opus decoder, it doesn't report any padding at
all.)
The Opus decoder (again libavcodec only tested) reports an initial
padding, but it appears to be too small, and it sounds right only with 2
packets discarded. So its status doesn't change.
I'm not sure why I need 2 frames for mp3, but with that value I had
success on the samples I tested.
Shitty ancient hack that wastes my time all the time.
demux.c: always return the coverart packet as soon as possible, and
don't let the backward demux state machine possibly stop it.
f_decoder_wrapper.c: mess with some shit until it somehow starts to
work. I think the old code tried to let it cleverly fall through so the
packet was processed "normally"; just make it run the "usual" code
instead.
Yay, more subtle state on top of this nightmarish, fragile state
machine. But this is what happens when you subvert the laws of nature.
This simple checks where playback should "resume" from when no packets
were returned to the decoder yet after the seek that initiated backward
playback. The main purpose is to process the first returned keyframe
range in the same way like all other ranges. This ensures that things
like preroll are included properly.
Before this commit, it could for example have happened that the start of
the first audio frame was slightly broken, because no preroll was
included. Since the audio frame is reversed before sending it to the
audio output, it would have added an audible discontinuity before the
second frame was played; all subsequent frames would have been fine.
(Although I didn't test and confirm this particular issue.)
In future, this could be useful for certain other things.
At least the condition for delaying the backstep seek becomes simpler
and more explicit.
Move the code that attempts to start demuxing up in dequeue_packet.
Before, it was not called when the stream was in back_restarting state.
This commit makes streams be in back_restarting state at initialization,
so the demuxer would never have started reading.
Likewise, we need to call back_demux_see_packets() right after seek in
case the seek was within the cache. (We don't bother with checking
whether it was a cached seek; nothing happens if it was a normal one.)
There is nothing else that would process these cached packets
explicitly, although coincidences could sporadically trigger it.
The check for back_restart_next in find_backward_restart_pos() now
decides whether to use this EOF special code. Since the backward
playback start state also sets this variable, we don't need some of
the complex checks in dequeue_packet() anymore either.
Make --audio-backward-overlap default to 2 for Opus. I have no idea why
this is needed. It seems to fix backward decoding though (going purely
by listening).
Normally, this should not be needed, since initial padding is completely
contained within the first packet (normally, and in the case I tested).
So the 2nd packet/frame should be fine, but for some unknown reason it
works only with the 3rd.
This seems more useful in general. This change also happens to fix a
miscounting of preroll packets when some of them were "rounded" away,
and which could make it stuck.
Also a simple intra-refresh encode with x264 (and muxed to mkv by it)
seems to work now. I guess I misinterpreted earlier results.
Backstepping still could get "stuck" if the demuxer didn't seek far back
enough. This commit fixes getting stuck if playing backwards from the
end, and audio has ended much earlier than the video.
In commit "demux: fix initial backward demuxing state in some cases",
I claimed that the backward seek semantics ("snapping" backward in
normal seeking, unrelated to backward playing) would take care of
this. Unfortunately, this is not always quite true.
In theory, a seek to any position (that does not use SEEK_FORWARD, i.e.
backward snapping) should return a packet for every stream. But I have a
mkv sample, where audio ends much earlier than video. Its mkvmerge
created index does not have entries for audio packets, so the video
index is used. This index has its last entry somewhere close after the
end of audio. So no audio packets will be returned. With a "too small"
back_seek_size, the demuxer will retry a seek target that ends up in
this place forever. (This does not happen if you use --index=recreate.
It also doesn't happen with libavformat, which always prefers its own
index, while mpv's internal mkv demuxer strictly prefers the index from
the file if it can be read.)
Fix this by adding the back_seek_size every time we fail to see enough
packets. This way the seek step can add up until it works.
To prevent that back_seek_pos just "runs away" towards negative infinity
by subtracting back_seek_size every time we back step to undo forward
reading (e.g. if --no-cache is used), readjust the back_seek_pos to the
lowest known resume position. (If the cache is active, kf_seek_pts can
be used, but to work in all situations, the code needs to grab the
minimum PTS in the keyframe range.)
Just rearranging shit. Setting SEEK_HR for backstep seeks actually
doesn't have much meaning, but disables the weird audio snapping for
"keyframe" seeks, and I don't know it's late.
This code used to be simpler, but now it's enough that it should be
factored into a single function.
Both uses of the new function are annoyingly different. The first use is
the special case when a decoder tries to read packets, but the demuxer
doesn't see any (like mp4 files with sparse video packets, which
actually turned out to be chapter thumbnail "tracks"). Then the other
stream queues will overflow, and the stream with no packets is marked
EOF to avoid stalling playback.
The second case is when the demxuer returns global EOF.
It would be more awkward to have the loop iterating the streams in the
function, because then you'd need a weird parameter to control the
behavior.
Just "mpv file.mkv --play-direction=backward" did not work, because
backward demuxing from the very end was not implemented. This is another
corner case, because the resume mechanism so far requires a packet
"position" (dts or pos) as reference. Now "EOF" is another possible
reference.
Also, the backstep mechanism could cause streams to find different
playback start positions, basically leading to random playback start
(instead of what you specified with --start). This happens only if
backstep seeks are involved (i.e. no cached data yet), but since this is
usually the case at playback start, it always happened. It was racy too,
because it depended on the order the decoders on other threads requested
new data. The comment below "resume_earlier" has some more blabla.
Some other details are changed.
I'm giving up on the "from_cache" parameter, and don't try to detect the
situation when the demuxer does not seek properly. Instead, always seek
back, hopefully some more.
Instead of trying to adjust the backstep seek target by a random value
of 1.0 seconds. Instead, always rely on the random value provided by the
user via --demuxer-backward-playback-step. If the demuxer should really
get "stuck" and somehow miss the seek target badly, or the user sets the
option value to 0, then the demuxer will not make any progress and just
eat CPU. (Although due to backward seek semantics used for backstep
seeks, even a very small seek step size will work. Just not 0.)
It seems this also fixes backstepping correctly when the initial seek
ended at the last keyframe range. (The explanation above was about the
case when it ends at EOF. These two cases are different. In the former,
you just need to step to the previous keyframe range, which was broken
because it didn't always react correctly to reaching EOF. In the latter,
you need to do a separate search for the last keyframe.)
In this scenario, the demuxer will output timestamps offset by the codec
delay (e.g. negative timestamps at the start; mkv simulates those), and
the trimming in the decoder (often libavcodec, but ad_lavc.c in our
case) will adjust the timestamps back (e.g. stream actually starts at
0).
This offset needs to be taken into account when seeking. This worked in
the uncached case. (demux_mkv.c is a bit tricky in that the index is
already in the offset space, so it compensates even though the seek call
does not reference codec_delay.) But in the cached case, seeks backwards
did not seek enough, and forward they seeked too much.
Fix this by adding the codec delay to the index search. We need to get
"earlier" packets, so e.g. seeking to position 0 really gets the initial
packets with negative timestamps.
This also adjusts the seek range start. This is also pretty obvious: if
the beginning of the file is cached, the seek range should start at 0,
not a negative value. We compare 0-based timestamps to it later on.
Not sure if this is the best approach. I also could have thought
about/checked some corner cases harder. But fuck this shit.
Not fixing duration (who cares) or end trimming, which would reduce the
seek range and duration (who cares).
Only timestamps that enter or leave the demuxer API should be adjusted
by ts_offset (which is usually the start time). queue_seek() is also
used by backward demux seeks, which uses an internal timestamp.
See manpage additions. This is a huge hack. You can bet there are shit
tons of bugs. It's literally forcing square pegs into round holes.
Hopefully, the manpage wall of text makes it clear enough that the whole
shit can easily crash and burn. (Although it shouldn't literally crash.
That would be a bug. It possibly _could_ start a fire by entering some
sort of endless loop, not a literal one, just something where it tries
to do work without making progress.)
(Some obvious bugs I simply ignored for this initial version, but
there's a number of potential bugs I can't even imagine. Normal playback
should remain completely unaffected, though.)
How this works is also described in the manpage. Basically, we demux in
reverse, then we decode in reverse, then we render in reverse.
The decoding part is the simplest: just reorder the decoder output. This
weirdly integrates with the timeline/ordered chapter code, which also
has special requirements on feeding the packets to the decoder in a
non-straightforward way (it doesn't conflict, although a bugmessmass
breaks correct slicing of segments, so EDL/ordered chapter playback is
broken in backward direction).
Backward demuxing is pretty involved. In theory, it could be much
easier: simply iterating the usual demuxer output backward. But this
just doesn't fit into our code, so there's a cthulhu nightmare of shit.
To be specific, each stream (audio, video) is reversed separately. At
least this means we can do backward playback within cached content (for
example, you could play backwards in a live stream; on that note, it
disables prefetching, which would lead to losing new live video, but
this could be avoided).
The fuckmess also meant that I didn't bother trying to support
subtitles. Subtitles are a problem because they're "sparse" streams.
They need to be "passively" demuxed: you don't try to read a subtitle
packet, you demux audio and video, and then look whether there was a
subtitle packet. This means to get subtitles for a time range, you need
to know that you demuxed video and audio over this range, which becomes
pretty messy when you demux audio and video backwards separately.
Backward display is the most weird (and potentially buggy) part. To
avoid that we need to touch a LOT of timing code, we negate all
timestamps. The basic idea is that due to the navigation, all
comparisons and subtractions of timestamps keep working, and you don't
need to touch every single of them to "reverse" them.
E.g.:
bool before = pts_a < pts_b;
would need to be:
bool before = forward
? pts_a < pts_b
: pts_a > pts_b;
or:
bool before = pts_a * dir < pts_b * dir;
or if you, as it's implemented now, just do this after decoding:
pts_a *= dir;
pts_b *= dir;
and then in the normal timing/renderer code:
bool before = pts_a < pts_b;
Consequently, we don't need many changes in the latter code. But some
assumptions inhererently true for forward playback may have been broken
anyway. What is mainly needed is fixing places where values are passed
between positive and negative "domains". For example, seeking and
timestamp user display always uses positive timestamps. The main mess is
that it's not obvious which domain a given variable should or does use.
Well, in my tests with a single file, it suddenly started to work when I
did this. I'm honestly surprised that it did, and that I didn't have to
change a single line in the timing code past decoder (just something
minor to make external/cached text subtitles display). I committed it
immediately while avoiding thinking about it. But there really likely
are subtle problems of all sorts.
As far as I'm aware, gstreamer also supports backward playback. When I
looked at this years ago, I couldn't find a way to actually try this,
and I didn't revisit it now. Back then I also read talk slides from the
person who implemented it, and I'm not sure if and which ideas I might
have taken from it. It's possible that the timestamp reversal is
inspired by it, but I didn't check. (I think it claimed that it could
avoid large changes by changing a sign?)
VapourSynth has some sort of reverse function, which provides a backward
view on a video. The function itself is trivial to implement, as
VapourSynth aims to provide random access to video by frame numbers (so
you just request decreasing frame numbers). From what I remember, it
wasn't exactly fluid, but it worked. It's implemented by creating an
index, and seeking to the target on demand, and a bunch of caching. mpv
could use it, but it would either require using VapourSynth as demuxer
and decoder for everything, or replacing the current file every time
something is supposed to be played backwards.
FFmpeg's libavfilter has reversal filters for audio and video. These
require buffering the entire media data of the file, and don't really
fit into mpv's architecture. It could be used by playing a libavfilter
graph that also demuxes, but that's like VapourSynth but worse.
The demuxer layer can start a thread to decouple the rest of the player
from blocking I/O (such as network accesses). But this particular
function does not support running with the thread enabled. The mutex use
within it is only since thread_work() may temporarily unlock the mutex,
and unlocking an unlocked mutex is not allowed. Most of the rest of the
code still does proper locking, even if it's pointless and effectively
single-threaded.
To make this look slightly cleaner, extend the mutex around the rest of
the code (like threaded code would have to do). This is mostly a
cosmetic change.
The demuxer cache benefits slightly from knowing where the current file
or stream begins. For example, seeking "left most" when the start is
cached would not trigger a low level seek (which would be followed by
messy range joining when it notices that the newly demuxed packets
overlap with an existing range).
Unfortunately, since multimedia is so crazy (or actually FFmpeg in its
quite imperfect attempt to be able to demux anything), it's hard to tell
where a file starts. There is no feedback whether a specific seek went
to the start of the file. Packets are not tagged with a flag indicating
they were demuxed from the start position. There is no index available
that could be used to cross-check this (even if the file contains a full
and "perfect" index, like mp4). You could go by the timestamps, but who
says streams start at 0? Streams can start somewhere at an extremely
high timestamps (transport streams like to do that), or they could start
at negative times (e.g. files with audio pre-padding will do that), and
maybe some file formats simply allow negative timestamps and could start
at any negative time. Even if the affected file formats don't allow it
in theory, they may in practice. In addition, FFmpeg exports a
start_time field, which may or may not be useful. (mpv's internal mkv
demuxer also exports such a field, but doesn't bother to set it for
efficiency and robustness reasons.)
Anyway, this is all a huge load of crap, so I decided that if the user
performs a seek command to time 0 or earlier, we consider the first
packet demuxed from each stream to be at the start of the file. In
addition, just trust the start_time field. This is the "shitty" part of
this commit.
One common case of negative timestamps is audio pre-padding. Demuxers
normally behave sanely, and will treat 0 as the start of the file, and
the first packets demuxed will have negative timestamps (since they
contain data to discard), which doesn't break our assumptions in this
commit. (Although, unfortunately, do break some other demuxer cache
assumptions, and the first cached range will be shown as starting at a
negative time.)
Implementation-wise, this is quite simple. Just split the existing
initial_state flag into two, since we want to deal with two separate
aspects. In addition, this avoids the refresh seek on track switching
when it happens right after a seek, instead of only after opening the
demuxer.
There were 3 packet reading functions: the "old" demux_read_packet()
that blocked (leftover from MPlayer times, but was still used until
recently by some obscure code), the "new" demux_read_packet_async(), and
the special demux_read_any_packet(), that is used by pseudo-demuxers
like demux_edl.
The first two could be used both in threaded and un-threaded mode. This
made 5 cases in total. Some bits of logic was spread across all of them.
Unify the logic. A recent commit made demux_read_packet() private, and
the code for it in threaded mode disappears. The difference between
threaded and un-threaded is minimized.
It's possible that this commit causes random regression. Enjoy.
There are 3 packet reading functions in the demux API, which all
function completely differently. One of them, demux_read_packet(), has
only 1 caller, which is in dec_sub.c. Change this caller to use
demux_read_packet_async() instead. Since it really wants to do a
blocking call, setup some proper waiting. This uses mp_dispatch_queue,
because even though it's overkill, it needs the least code.
In practice, waiting actually never happens. This code is only called on
code paths where everything is already read into memory (libavformat's
subtitle demuxers simply behave this way). It's still a bit of a
"coincidence", so implement it properly anyway.
If suubtitle decoder init fails, we still need to unset the demuxer
wakeup callback. Add a sub_destroy() call to the failure path. This also
happens to fix a missed pthread_mutex_destroy() call (in practice this
was a nop, or a memory leak on BSDs).
I'm not sure about this, but it looks like a bug. If a stream didn't
have packets, but the joined range does, the stream should obviously
read the packets added by the joined range. Until now, due to
reader_head being NULL, reading was only resumed if a _new_ packet was
added by actual demuxing (in add_packet_locked()), which means the
stream would suddenly skip ahead, past the original end of the joined
range.
Change it so that it will pick up the new range.
Also, clear the skip_to_keyframe flag. Nothing useful can come from this
flag being set; in the first place, the first packet of a range (that
isn't the current range) should start with a keyframe. Some code
probably enforced it (although it's fuzzy).
Completely untested.
When doing a seek to the end of the cache, ds->skip_to_keyframe can be
set to true. Then some packets passed to add_packet_locked() may have to
be skipped. In some aspects, the skipped packet was still treated as if
it was going to be returned to the reader.
It almost doesn't matter though: it only caused a redundant wakeup_ds()
call, and could pass the packet to the stream recorder. Fix it anyway.
This fixes that there were weird delay ("buffering") when seeking into
the last part of a seekable range. The exact case which triggers it if
SEEK_FORWARD is used, and the seek pts is after the second-last
keyframe, but before the end of the range. In that case,
find_seek_target() returned NULL, and the cache layer waited until the
_next_ keyframe the underlying demuxer returned until resuming playback.
find_seek_target() returned NULL, because the last keyframe had
kf_seek_pts unset. This field contains the lowest PTS in the packet
range from the keyframe until the next keyframe (or EOF). For normal
seeks, this is needed because keyframes don't necessarily have the
minimum PTS in the packet range, so it needs to be computed by waiting
for all packets until the next keyframe (or EOF).
Strictly speaking, this behavior was correct, but it meant that the
caller would set ds->skip_to_keyframe, which waits for the next newly
demuxed keyframe. No packets were returned to the decoder until this
happened, usually resulting in the frontend entering "buffering" mode.
What it really needs to do is returning the last keyframe in the cache.
In this situation, the seek target points in the middle of the last
completely cached packet range (as delimited by keyframes), and
SEEK_FORWARD is supposed to skip to the next keyframe. This is in line
with the basic assumptions the packet cache makes (e.g. the keyframe
flag means it's possible to start decoding, and the frames decoded from
it and following packets will strictly have PTS values above the
previous keyframe range). This means in this situation the kf_seek_pts
value doesn't matter either.
So fix this situation by explicitly detecting it and then returning the
last cached keyframe.
Should the search loop look at all packets, instead of only keyframe
ones? This would mean it can know that it's within the last keyframe
range (without looking at queue->seek_end). Maybe this would be a bit
more natural for the SEEK_FORWARD case, but due to PTS reordering it
doesn't sound like a useful thing to do.
Should skip_to_keyframe be checked by the code that sets kf_seek_pts to
a known value? This wouldn't help too much; the frontend would still go
into "buffering" mode for no reason until the packet range is completed,
although it would resume from the correct range.
Should a NULL return always unconditionally use keyframe_latest? This
makes sense because the seek PTS is usually already in the cached range,
so this is the only case that should happen. But there are scary special
cases, like sparse subtitle streams, or other uses of find_seek_target()
which could be out of range now or in future. Basically, don't "risk"
it.
One other potential problem with this is that the "adjust seek target"
code will be disabled in this case. It checks kf_seek_pts, and if it's
unset, the adjustment is not done. Maybe this could be changed to use
the queue's seek_end time, but I'm not sure if this is fully kosher. On
the other hand, I think the main use for this adjustment is with
backwards seeks, so this shouldn't matter.
A previous commit dealing with audio/video stream merging mentioned how
seeking forward entered "buffering" mode for unknown reasons; this
commit fixes this issue.
demux_timeline doesn't do any transport accesses itself. The slave
demuxers do this (these will actually access the stream layer and
perform e.g. network accesses). As a consequence, demux_timeline always
reported 0 bytes read, and network speed display didn't work.
Fix this by awkwardly reporting the amount of read bytes upwards. This
is not very nice, and requires explicit calls whenever the slave "might"
have read data.
Due to the way the reporting is done, it only works if the slaves do not
run demuxer threads, which makes things even less nice. (Fortunately
they don't anyway, because it would be a waste of resources.) Some
identifiers contain the word "hack" as a warning.
Some of the stupidity comes from the fact that demux.c itself resets the
stats randomly in order to calculate the bytes_per_second value, which
is useless for a slave, but of course is still done, because demux.c
itself is not aware of whether it's on the slave or top-level layer.
Unfortunately, this must do.
In theory, the demuxer thread/cache layer should be separated from
demuxer implementations. This would get rid of all the awkwardness and
nonsense. For example, the only threading involved would be the caching
layer, completely separate from demuxers themselves. It'd be the only
thing calculates speed rates for the player frontend, too (instead of
doing it for each demuxer, even if unused).
It was an ugly hack, and the next commit will make it even uglier.
Slightly reduce the ugliness to prevent death of too many brain cells,
though it's still an ugly hack.
The cleanup is really minor, but I guess the following commit would be
much worse otherwise. In particular, this commit checks accesses
(instead of having a public field with evil access rules), which should
avoid misunderstandings and incorrect use. Strictly speaking, the added
field is redundant, but the next commit complicates it a bit.
The only thing left is the notification for track switching. Just get
rid of that.
There's probably no real reason to get rid of control(), but why not. I
think I was actually trying to do some real work but fuck that.
Subtitles (and a few other file types, like playlists) are not streamed,
but fully read on opening. This means keeping the file handle or network
socket open is a waste of resources and could cause other weird
behavior. This is why there's a hack to close them after opening.
Change this hack to make the demuxer itself do this, which is less
weird. (Until recently, demuxer->stream ownership was more complex,
which is why it was done this way.)
There is some evil shit due to a huge ownership/lifetime mess of various
objects. Especially EDL (the currently only nested demuxer case)
requires being careful about mp_cancel and passing down stream pointers.
As one defensive programming measure, stop accessing the "stream"
variable in open_given_type(), even where it would still work. This
includes removing a redundant line of code, and removing the peak call,
which should not be needed anymore, as the remaining demuxers do this
mostly correctly.
I always wanted to get rid of this, because it makes the ownership rules
for the stream pointer really awkward. demux_edl.c was the only
remaining user of this. Replace it with a semi-clever idea: the init
segment shit can be used to pass the "file" contents as memory block,
and "memory://" itself provides an empty stream. I have no idea if this
actually works, because I didn't immediately find a test stream (would
have to be some youtube DASH shit).
Instead of going through those weird DEMUXER_CTRLs, query this
information directly. I'm not sure which kind of brain damage made me
use CTRLs for these. Since there are no other DEMUXER_CTRLs that make
sense for the frontend, remove the remaining infrastructure for them
too.
The stream size return was the only thing that still required doing
STREAM_CTRLs from frontend through the demuxer layer. This can be done
much easier, so rip it out. Also rip out the now unused infrastructure
for STREAM_CTRLs via demuxer layer.
Apparently this was so that when playing a video file from a .rar file,
it would load external subtitles with the same name (instead of looking
for mpv's rar:// mangled URL). This was requested on github almost 5
years ago. Seems like a weird feature, and I don't care. Drop it,
because it complicates some in progress change.
Linux analog TV support (via tv://) was excessively complex, and
whenever I attempted to use it (cameras or loopback devices), it didn't
work well, or would have required some major work to update it. It's
very much stuck in the analog past (my favorite are the frequency tables
in frequencies.c for analog TV channels which don't exist anymore).
Especially cameras and such work fine with libavdevice and better than
tv://, for example:
mpv av://v4l2:/dev/video0
(adding --profile=low-latency --untimed even makes it mostly realtime)
Adding a new input layer that targets such "modern" uses would be
acceptable, if anyone is interested in it. The old TV code is just too
focused on actual analog TV.
DVB is rather obscure, but has an active maintainer, so don't remove it.
However, the demux/stream ctrl layer must go, so remove controls for
channel switching. Most of these could be reimplemented by using the
normal method for option runtime changes.
This removes anything related to DVD/BD/CD that negatively affected the
core code. It includes trying to rewrite timestamps (since DVDs and
Blurays do not set packet stream timestamps to playback time, and can
even have resets mid-stream), export of chapters, stream languages,
export of title/track lists, and all that.
Only basic seeking is supported. It is very much possible that seeking
completely fails on some discs (on some parts of the timeline), because
timestamp rewriting was removed.
Note that I don't give a shit about optical media. If you want to watch
them, rip them. Keeping some bare support for DVD/BD is the most I'm
going to do to appease the type of lazy, obnoxious users who will care.
There are other players which are better at optical discs.
Manual changes done:
* Merged the interface-changes under the already master'd changes.
* Moved the hwdec-related option changes to video/decode/vd_lavc.c.
The seek range update was to early and did not take the removed head
packets into account. And therefore missed that the queue was not
BOF anymore.
This led to not be able to backward seek before the first packet of
the first seek range.
Fix it by moving the seek range update after the possible removal and
the change of the BOF flag.
Fixes: #6522
Commit e392d6610d modified the native
demuxer to use track gain as a fallback for album gain if the latter is
not present. This commit makes functionally equivalent changes in the
libavformat demuxer.
If the number of chapters is 0, the chapter list can be NULL. clang
complains that we pass NULL to qsort(). This is yet another pointless UB
that exists for no reason other than wasting your time.
--record-file is nice, but only sometimes. If you watch some sort of
livestream which you want to record, it's actually much nicer not to
record what you're currently "seeing", but anything you're receiving.
wm4
Tom Yan
Jan Ekström
Gunnar Marten
Benjamin Barenblat
sfan5
Niklas Haas