| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
| |
Some mpv code once needed this, but it was removed in 2015.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
| |
injecting the Apple Main Thread Checker via
DYLD_INSERT_LIBRARIES=libMainThreadChecker.dylib identified several
problems that needed fixing.
|
| |
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
| |
Much more verbose, but on the other hand format_note is useless for the
alphabetic site with fragmented DASH streams.
|
|
|
|
|
| |
"format_note" normally contains a semi-informative description of the
format. But some extractors, confusingly, have it in the "format" field.
|
|
|
|
|
| |
E.g. soundcloud. While it still worked, not having the audio codec was
pretty annoying.
|
|
|
|
|
|
| |
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.
|
|
|
|
| |
Well, didn't help much in the case I was interested it.
|
| |
|
| |
|
|
|
|
| |
Should not be needed anymore. In fact, it's probably dangerous.
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
| |
Also, the forced flag (and in the future, potentially a number of other
flags not implemented yet). See next commit for purpose.
|
| |
|
| |
|
|
|
|
|
| |
This was done for each media type, so muxed tracks had it twice, which
logged a dumb warning. Move it out of the per-media type loop.
|
|
|
|
|
|
|
|
| |
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.)
|
|
|
|
|
| |
I think this is unnecessary, and at worst done by youtube-dl itself
(didn't check).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
| |
Make it a proper list, instead of a paragraph soup.
|
|
|
|
|
|
| |
(Or if it's about HLS, just "muxed"/multiplexed streams.)
This only affects all_formats=yes,skip_muxed=no modes.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
| |
It even has a typo.
|
|
|
|
| |
If available.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.)
|
|
|
|
|
| |
Shouldn't have any consequences. Probably makes the user-visible order
more stable.
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
| |
Because the --hls-bitrate option takes the same unit.
|
|
|
|
|
| |
In particular, all_formats description split away the example section of
an unrelated option, so move that to its proper place.
|
|
|
|
|
|
|
|
|
|
| |
This is just a more convenient way to start IPC client scripts per mpv
instance.
Does not work on Windows, although it could if the subprocess and IPC
parts are implemented (and I guess .exe/.bat suffixes are required).
Also untested whether it builds on Windows. A lot of other things are
untested too, so don't complain.
|
|
|
|
| |
Basically, UNIX sucks. (Not as much as the other POS of course.)
|
|
|
|
|
|
| |
This is UNIX-only code, and this function has been in POSIX since
forever. Even Android has it. The test should be unnecessary, so remove
it.
|
|
|
|
|
|
|
|
|
| |
Pretty worthless I guess. I only tested one site (and 2 videos), it's
somewhat likely that it will break with other sites. Even if you leave
the option disabled (the default).
Slightly related to #3548. This will allows you to use the bitrate
stream selection mechanism, that was added for HLS, with normal videos.
|
|
|
|
| |
Might be helpful for... whatever.
|
|
|
|
|
| |
Aka hls-bitrate. In turn, remove the demux_lavf.c hack, which made the
track title use this.
|
| |
|
|
|
|
|
|
| |
Neither does it (directly) mess with filters, nor does it return a bool.
As noticed by a comment in #6333.
|
|
|
|
| |
example config and default term-status-msg
|
|
|
|
|
|
|
| |
Caused build failures with still supported FFmpeg versions. It's
unreferenced, so it's not needed.
Fixes: #7471
|
|
|
|
|
| |
It's still deprecated, but I guess users who preferred typing a space
instead of a '=' can use it.
|
|
|
|
| |
This was sort of asymmetric and annoying.
|
|
|
|
|
|
|
|
|
|
|
| |
While paused, the decoders typically stop reading data from the demuxer.
But for some reason, the file size is returned as a public field in
struct demuxer (wat...), and updated only when the packet reading
function is called. This caused the file size property to always return
the same value when paused, even though the demuxer thread was reading
new data, and the internal file size was updated.
Fix with a simple hack.
|
|
|
|
| |
With the last 3 commits, this caching should be completely unnecessary.
|
|
|
|
|
|
|
|
|
|
| |
Instead of every packet. Doing it every packet led to the performance
regression mentioned in the fstat() commit. This should now be over, but
out of being careful, don't query the file size that often. This is only
used for user interface things, so this should not cause any problems.
For the sake of leaving the code compact, abuse another thing that is
updated only every second (speed statistics).
|
|
|
|
| |
Equivalent, just slightly more convenient for the following change.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It appears using lseek() to seek to the end and back to determine file
size is inefficient in some cases.
With CIFS, this restores the performance regression that happened when
the stream cache was removed (which called read() from a thread). This
is probably faster than the old code too, because it's the seeking that
was slowing down CIFS.
According to the user who tested this, the size caching does not help
with fstat() (although it did with the old method).
Fixes: #7408, #7152
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The previous method for this sucked: for every launched detached
process, it started a thread, which then would leak if the launched
process didn't end before the player uninitialized. This was very racy
(although I bet the race condition wouldn't trigger in a 100 years), and
wasteful (threads aren't a cheap resource).
Implement it for POSIX directly. posix_spawn() has no direct support for
this, so we need to do it ourselves with fork(). We could probably do it
without fork(), and attempt to collect the PID in another thread. But
then we'd either have a waiting thread again, or we'd need to do an
unsafe waitpid(-1, ...) call. (POSIX process management sucks so badly,
how did they even manage this. Hopefully I'm just missing something, but
I'm not.) So now we depend on both posix_spawn() _and_ fork(), isn't it
fun?
Also call setsid(), to essentially detach the child process from the
terminal. (Otherwise it can receive various signals from the terminal,
which is probably not what you want.) posix_spawn() adds
POSIX_SPAWN_SETSID in newer POSIX releases, but we don't want to rely on
this yet.
The posix_spawnp() call is duplicated, but this is better than somehow
trying to unify the code paths.
Only somewhat tested, so enjoy the bugs.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Introduce mp_subprocess() and related definitions. This is a bit more
flexible than the old stuff. This may or may not be used for a more
complicated feature that involves starting processes, and which would
require more control.
Only port subprocess-posix.c to this API. The player still uses the
"old" API, so for win32 and dummy implementations, the new API is simply
not available, while for POSIX, the old APIs are emulated on top of the
new one. I'm hoping the win32 code can be ported as well, so the ifdefs
in subprocess.c can be dropped, and the player can (if convenient or
needed) use the new API.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Filters that fail to create are not supposed to do this. Generally it
should happen in process() only.
This fixes the previous commit. If a filter could not be created, it
"trashed" the wrapper filter with the failure. (Even if the wrapper were
to handle were to handle failures of sub-filter, it couldn't handle init
|