Make the demuxer cache the default, reduce stream cache #5250
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I added another commit, that actually partially reverts the first commit. I'd like to keep the first commit anyway, so the commit history documents this attempt of a solution. |
|
Ping. Does anyone disagree with the direction this is taking? |
added 6 commits
December 22, 2017 01:54
This fixes weird behavior in the following case: - open a file - make sure the max. demuxer forward cache is smaller than the file's video track - make sure the max. readahead duration is larger than the file's duration - disable the audio track - seek to the beginning of the file - once the cache has filled enable the audio track - a queue overflow warning should appear (- looking at the seek ranges is also interesting) The queue overflow warning happens because the packed queue for the video track will use up the full quota set by --demuxer-max-bytes. When the audio track is enabled, reading an audio packet would technically overflow the packet cache by the size of whatever packet is read next. This means the demuxer signals EOF to the decoder, and once playback has consumed enough video packets so that audio packets can be read again, the decoder resumes from EOF. This interacts badly with A/V synchronization and the whole thing can randomly crap itself until audio has fully recovered. We didn't care about this so far, but we want to raise the readahead duration to something very high, so that the demuxer cache is fully used. This means this case can be hit quite quickly by switching audio or subtitle tracks, and is not really an obscure corner case anymore. Fix this by always losing all cache. Since the cache can't be used anyway until the newly selected track has been read, this is not much of a disadvantage. The only thing that could be brought up is that unselecting the track again could resume operation normally. (Maybe this would be useful if network died completely without chance of recovery. Then you could watch the already buffered video anyway by deselecting the audio track again.) But given the headaches, this seems like the better solution. Unfortunately this requires adding new new strange fields and strangely fragmenting state management functions again. I'm sure whoever works on this in the future will hate me. Currently it seems like the lesser evil, and much simpler and robust than the other potential solutions. In case this needs to be revisited, here is a reminder for readers from the future what alternative solutions were considered, without those disadvantages: A first attempted solution allowed the demuxer to buffer some additional packets on track switching. This would allow it to read enough data to feed the decoder at least. But it was still awkward, as it didn't allow the demuxer to continue prefetching the newly selected track. It also barely worked, because you could make the forward buffer "over full" by seeking back with seekable cache enabled, and then it couldn't read packets anyway. As alternative solution, we could always demux and cache all tracks, even if they're deselected. This would also not require a network-level seek for the "refresh" logic (it's the thing that lets the video decoder continue as if nothing happened, while actually seeking back in the stream to get the missing audio packets, in the case of enabling a previously disabled audio track). But it would also possibly waste network and memory resources, depending on what the user actually wants. A second solution would just account the queue sizes for each stream separately. We could freely fill up the audio packet queue, even if the video queue is full. Since the demuxer API returns interleaved packets and doesn't let you predict which packet type comes next, this is not as simple as it sounds, but it'd probably tie in nicely with the "refresh" logic. A third solution would be removing buffered video packets from the end of the packet queue. Since the "refresh" logic gets these anyway, there is no reason to keep them if they prevent the audio packet queue from catching up with the video one. But this would require additional logic, would interact badly with a bunch of other corner cases. And as far as the code goes, it's rather complex, because all the logic is written with FIFO behavior in mind (including the fact that the packet queue is a singly linked list with no backwards links, making removal from the end harder).
Set it to 10 hours, which is practically unlimited. (Avoiding use of "inf", since that might interact strangely with the option parser and such.)
Reduce it from 75MB in both directions (forward/backwards) to 10MB each. The stream cache is kind of becoming useless in favor of the demuxer cache. Using both doesn't make much sense, because they will contain duplicated data for no reason. Still leave it at 10MB, which may help with mp4 a bit. libavformat's mp4 demuxer tends to seek too much, so we try to avoid triggering network level seeks by having some caching in the stream layer.
I don't want to add another field to display stream and demuxer cache separately, so just add them up. This strangely makes sense, since the forward buffered stream cache amount consists of data not read by the demuxer yet. (If the demuxer cache has buffered the full stream, the forward buffered stream cache amount is 0.)
Same as previous commit, but for the OSC. (A bit of a waste to request demuxer-cache-state at least twice per frame, but the OSC queries so many properties it probably doesn't matter anymore.)
This log line tells us why the demuxer is trying to read more, which us useful when debugging queue overflows. Probably barely useful, but I think keeping that flag separately also makes the code slightly easier to understand.
|
OK, squashed the 2 commits (and separated out an unrelated change) and rebased. |
|
Huge 👍 to this direction! |
mia-0
pushed a commit
that referenced
this pull request
Dec 26, 2017
Show total cache as well as demuxer cache separately. This adjusts the presented values to be consistent with status line and OSC modifications made in #5250
Argon-
pushed a commit
to Argon-/mpv-stats
that referenced
this pull request
Dec 26, 2017
Show total cache as well as demuxer cache separately. This adjusts the presented values to be consistent with status line and OSC modifications made in mpv-player/mpv#5250
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes have the goal to make the demuxer cache the default for network operation.
This raises the demuxer readahead limit to practically unlimited (well, 10 hours), which either enables reading the entire stream into memory, or limits it by the given byte limits (currently 400MB, per demuxer).
This also includes a small fix for track switching awkwardness, although it doesn't fix it fully. This applies to situations where the demuxer cache is maxed out on the byte limit, and you enable a stream (details see commit message). It mitigates one worry about enabling this.
Finally, this includes the demuxer cache byte amount in the CLI status line and the OSC.
In general, the demuxer cache is less efficient than the stream cache. The stream cache is a simple byte based ringbuffer, while the demuxer cache is a linked list of demuxed packets, which uses a high number of memory allocations with lots of overhead. But it works much better for certain streaming protocols. (There are simpler solutions, but libavformat's abstractions are very unhelpful, so they are actually harder to implement.)