A negative subtitle delay means that subtitles from the future should be
shown earlier. With muxed subtitles, subtitle packets are demuxed along
with audio and video packets. But since they are demuxed "lazily",
nothing guarantees that subtitle packets from the future are available
in time.
Typically, the user-observed effect is that subtitles do not appear at
all (or too late) with large negative --sub-delay values, but that using
--cache might fix this.
Make this behave better. Automatically extend read-ahead to as much as
needed by the subtitles. It seems it's the easiest to pass the subtitle
render timestamp to the demuxer in order to guarantee that everything is
read. This timestamp based approach might be fragile, so disable it if
no negative sub-delay is used.
As far as the player frontend part is concerned, this makes use of the
code path for external subtitles, which are not lazily demuxed, and may
already trigger waiting.
Fixes: #7484
Subtitle tracks are usually "lazy" (ds->eager=false), There are a number
of weird special cases associated with it. One of them is that they have
some sort of "temporary" EOF (to signal that there isn't a packet right
now, and the decoder should not block playback by waiting for more
packets). In a the next commit, I want to call mark_stream_eof() in case
of (some) of these temporary EOFs.
The problem is that mark_stream_eof() also calls the functions touched
by this commit. Basically they shouldn't do any complex work due to
these temporary EOFs (because they might happen very often). It turns
out that lazy tracks barely matter here: they do not extend the seek
range of a packet/EOF happens on them, they do not trigger seek range
joining, and they do not support backward demuxing.
This change should enable the following commit, while not causing any
behavior changes (i.e. bugs) with the current state.
args->client was deallocated if the FDs were closed and nothing
referenced it (IPC socket codes detected the closed sockets and
asynchronously killed the mpv_handle in args->client). The problem was
that args->log depended on it, and was also destroyed.
Fix this by duplicating the mp_log.
This has been part of the libmpv for a while, so the implementation in
the IPC code is quite simple: just pass the mpv_node representing the
value of the "command" field without further checks to
mpv_command_node().
The only problem are the IPC-specific commands, which essentially have
their own dispatch mechanism. They expect an array. I'm not going to
rewrite the dispatch mechanism, so these still work only with an array.
I decided make the other case explicit with cmd==NULL. (I could also
have set cmd=="", which would have avoided changing each if condition
since "" matches no existing command, but that felt dirty.)
I decided to make this explicit. The alternative would have been making
all commands asynchronous always, like a small note in the manpage
threatened. I think that could have caused compatibility issues.
As a design decision, this does not send a reply if an async command
started. This could be a good or bad idea, but in any case, it will make
async command look almost like synchronous ones, except they don't block
the IPC protocol.
The relatively recently added property update code has a race condition
when clients exit. It still tried to access mpv_handle during and after
it was destroyed.
The reason is that it unlocks the lock for the mpv_handle list (while
mpv_handle is locked), but nothing in mp_destroy_client() cares about
this case. The latter function locks mpv_handle only before/while it
makes sure all of its activity is coming to an end, such as asynchronous
requests, and property updates that are in progress. It did not include
the case when mp_client_send_property_changes() was still calling
send_client_property_changes() with mpv_handle locked.
Fix this by checking the mpv_handle.destroying field. This field can be
set only when mpv_handle is locked. While we're checking the lock, the
mpv_handle list is still locked, so at worst we might be at the point
before mp_destroy_client() locks the list again and finally destroys the
mpv_handle.
This is a hard to reproduce race condition which I spotted only once in
valgrind by chance, so the fix is unconfirmed.
The previous change ensured that these cannot fail anymore (much like in
original talloc). Change the APIs to not return a success value anymore,
to "cement" this.
The ta_ext_header was allocated on demand for allocations which have
child-allocations or destructors. In theory, it saved 2 words for every
TA leaf allocation. It had the very API-visible problem that setting a
parent or destructor could fail. (Although in most cases, the failure
was part of an allocation call anyway. Also, mpv code generally used the
early-failure variants, so it didn't matter.)
I think this was a bit too complex. These 2 words don't really matter;
if you have memory allocations where you are worried about overhead,
then these simply shouldn't use TA. Also, we never added new features to
TA that would have needed more "optional" header fields, which would
have justified the use of such a separately allocated header struct.
This uses quite straight-forward data structures. The only strange thing
is that ta_header.parent is NULL for most child allocations. That is
because we don't want to iterate over all children when the parent is
reallocated (yes, that is allowed, yes mpv makes use of it).
The new code has a few more special cases, because the list sentinel
concept isn't used anymore. Using it would have made the code more
unnatural/complex, because ta_ext_header doesn't exist anymore.
the actual character that made mpv crash is IDEOGRAPHIC COMMA
(U+3001, UTF-8: E3 80 81, 、) and that only in some specific
circumstances that could be reliably reproduced on my end.
using an NSString instead of the Swift String actually fixes that issues
even though they should technically do the exact same thing. i tested
all the other String initialisers, but they all had had the same issue.
this is kinda only a workaround till i can find a different way of
handling it.
this basically moves the remote command center to our mac events instead
of keeping it our Application, which is only available when started from
mpv itself. also make it independent of the NSApplication.
this also prevents a runtime crash
This was changed 6 years ago (444e583b6) and seemed to work fine. But it
does seem to cause issues with IceWM sometimes, while with StaticGravity
the problem is gone. Comparing both gravity values, reading the confused
source code comment, and reading the referenced commit message, I can't
determine what it even does, I just remove it.
Reproduction:
- start mpv in windowed mode, with 2 videos of different size
- switch to second video
- switch window with alt+tab
- switch back to mpv with alt+tab
- window moves to X=0
There's probably a better way to fix this. Please send a patch.
This was obviously nonsense. In Lua 5.1 this appeared to work correctly,
but it really turned "\." into "." (making the pattern accept any
character). The proper way is using "%" for escaping.
In all_formats mode, we've ignored what --ytdl-format did so far, since
we've converted the full format list, instead of just the formats
selected by youtube-dl.
But we can easily restore --ytdl-format behavior: just mark the selected
tracks as default tracks.
A parameter that is actually used is removed from the param_names[]
array, so we can report unused parameters. This also happened on
duplicate parameters, so adjust the warning to make it less confusing.
(In any case, you're not supposed to provide duplicate parameters.)
vbr and abr are the video and audio bitrates. Sometimes there is a weird
mix of any of them available, but in these cases, it's not good to fall
back to tbr if a specific track has no vbr/abr.
For example, the alphabetic site provides tbr only for the muxed
fallback stream, but using tbr would make the primitive mpv hls_bitrate
selection pick the compatibility stream for audio, because it appears to
have a higher bitrate than the other audio-only streams (because the
bitrate includes video). So we must not use tbr in this case.
On the other hand, formats coming from youtube-dl HLS master playlist
use will only have tbr set.
So as a heuristic, use the tbr only if it's the only bitrate available
in any track entry.
I don't think the skip_muxed option was overlay useful. While it was
nice to filter out the low quality muxed versions (as it happens on the
alphabetic site, I suspect it's compatibility stuff), it's not really
necessary, and just makes for another tricky and rarely used
configuration option. (This was different before muxed tracks were also
delay-loaded, and including the muxed versions slowed down loading.)
Add the force_all_formats option instead, which handles the HLS case.
Set it to true because they are also delay-loaded now, and don't slow
down startup as much.
Until now, delay-loading was for files with single tracks only
(basically what DASH and HLS like to expose, so adaptive streaming and
codec selection becomes easier - for sites, not for us). But they also
provide some interleaved versions, probably for compatibility. Until
now, we were forced to eagerly load it (making startup slightly slower).
But there is not much missing. We just need a way to provide multiple
metadata entries, and use them to represent each track.
A side effect is now that the "track_meta" header can be used for normal
EDL files too.
RTSP supports seeking, but at least the libavformat implementation makes
this dependent on runtime behavior. So you have to perform a seek, and
check if it fails. But even if you do this, the stream is interrupted
and restarted, and there seem to be other issues.
Assume that RTSP with unknown duration means it's a live stream, and
disable seeking in this case, as suggested by the issue reporter.
Fixes: #7472
Now this was stupid. To seek a source, it obviously has to be opened...
so just don't try to seek any unused source. If the track is actually
selected during playback, a seek to the correct position is performed
anyway.
These have ->segmented set (so the codec can be initialized properly),
but have no segment start or end times. This code was (probably) the
only thing which didn't handle this case.
If all streams were delay loaded, there was actually no duration present
at all in the EDL metadata. So the length was considered unknown by the
player frontend.
See manpage additions. We would have to extend delay_open to support
multiple sub-tracks (for audio and video), and we'd still don't know (?)
whether it might contain more than one stream each (thinking of HLS
master streams). And if it's a true interleaved file (such as a "normal"
mp4 file provided as fallback for more primitive players), we'd either
have to signal such "bundled" tracks, or waste bandwidth.
This restructures a lot. The if/else tree in add_single_video for format
selection was a bit annoying, so it's split into separate if blocks,
where it checks each time whether a URL was determined yet.
If a "format" has both audio and video codec set, it might contain both
audio and video. all_format assumes that each format is just a quality
variant containing a single track.
This seems to happen with sites that provide a HLS master URL.
youtube-dl tends to "butcher" it, and the result isn't very ideal. I
guess HLS "renditions" simply don't map well to youtube-dl's output
format and what mpv expects. Playing master HLS directly is also less
than ideal, because of libavformat's stupid probing.
Fix this by not using the delay-opening mechanism if it appears like we
detected such a case. Add a metadata override to set the track titles to
"muxed-N", to indicate that they form a single unit. (Mostly helpful for
testing.)
ytdl_hook.lua can do this with all_formats and when delay_open is used,
and if the source stream actually contains both audio and video. In this
case, it might accidentally hide a media type completely, or waste
bandwidth (if the stream has true interleaved audio/video). So it's
important to warn.
wm4
linkmauve
sfan5
der richter