In theory, it's better to keep the old value, because that's more
consistent with the logic of using change timestamps. With the current
code, the old value will probably never be used (instead it will fetch a
new value on every change), so this shouldn't make a difference in
practice.
In commit 065c307e8e, I broke everything. It seemed like a nice idea,
but it explicitly broke an assumption vo_libmpv were explicitly allowed
to make: that observing properties does not lock the core. The commit
did just that and locked the core for property updates. This made for
example mpv's own OSX backend freeze (it uses vo_libmpv for convenience
to make up for Apple's incredibly broken OpenGL shit).
I don't want to revert that commit just because vo_libmpv's design is
horrible. So instead add an optional asynchronous path, that is only
used if vo_libmpv is in use (best idea ever?).
Interestingly, this isn't so hard. It adds about 90 lines of code, which
are only run on OSX and libmpv users, so I don't have to care about the
crashes and weird behavior this might cause. It even worked on the first
try except for a quickly fixed memory leak. The code path can be tested
anywhere by just turning the uses_vo_libmpv condition into always true.
The atomic is out of laziness. Saves some thinking how to get around the
locking order.
The commit linked below added temporary unlocking to update_prop(),
which is indirectly called by mpv_wait_event(). If an unlock happens,
and no property change event is returned, we must re-poll the event
queue. Rechecking the state on unlocks is basically a standard
requirement for code using condition variables.
Untested beyond a simple test.
Fixes: 065c307e8e
Property change notification works by having the mpv core wake up all
clients observing a property when the property potentially changes. The
clients then read the property's value, and determine if there was an
actual change. (The latter part depends what the property returned for
the previous change notification, so it depends on the client, and
cannot be generated by the core itself.)
Until now, reading the property value was done in a pseudo-async way by
queuing a callback back to the core, running it there, and then waking
up the client thread again. I cannot comprehend why this was done in
such a complicated, fragile way. Maybe it's a leftover from times when
client.c had to do this (in short, because properties could access
vo_opengl, which has thread-local state).
One past idea was to make the implementation of true async properties
easier (for which you would need such a state machine anyway). But they
don't exist yet, and I doubt the current mess would be really helpful
when actually implementing them.
Simplify this, and run the update in the client's thread directly. In
addition to the fundamental change, many roundabout things can be
removed as a consequence.
Unfortunately, I noticed that lock order issues force you to release
ctx->lock before doing so, which makes things more complex due to
possible concurrent mpv_unobserve_property() calls. Solve this by
removing properties lazily, which means you may have to do multiple
mpv_wait_event() calls before the property entry is actually destroyed.
This should not matter in practice, and does not affect the semantics.
It could also cause "leaks" by observing/unobserving properties in a
loop, without ever calling mpv_wait_event(). Just don't do this, duh.
(I considered making this dependent on whether the previous
mpv_wait_event() call returned the property being removed, but a
separate code path seemed too complicated. I also considered copying the
name and property data when returning a MPV_EVENT_PROPERTY_CHANGE, but
actually this doesn't solve the problem of update_prop() being
interrupted by mpv_unobserve_property(); there are ways around it, but I
just said no.)
This was made using the cowboy coding software engineering methodology.
If you find any bugs, keep them yourself.
Apparently this was only added and used for cache update stuff, which
was changed in commit 8dbc93a94c. Now it's unused, messy, ugly, and
is in the way, so get rid of it.
Since a track may not be selected twice, it makes sense e.g. for
secondary subtitles to select the next best match and avoid the
duplicate selection.
This allows for example `--slang=en,ja --secondary-sid=auto` to select
'en' as primary and 'ja' as secondary without needing to know the actual
sid for 'ja'.
On a audio/video desync by more than 0.5 seconds, display-sync mode was
disabled, and not enabled again (until playback restart, e.g. a seek).
The idea was that it this only happens when this playback mode is broken
and can't perform well anyway (A/V desync is a clear indication that
something is very wrong). Instead of behaving like a god damn POS, it
should revert to the more robust audio-sync mode.
Unfortunately, this could happen sporadically due to temporary system
performance problems, such as toggling fullscreen. Users didn't like
this, and asked for a function to disable it, or to recover in some
other way.
This mechanism is questionable anyway. If an ignorant user enables
display-sync, and encounters problems with it (without being able to
determine that display-sync is messing up), the player will still behave
like a POS on every playback, and even after every seek. It might
actually be helpful to fail more consistently. Also, I've found that
it's sill relatively reliable anyway even without this mechanism.
So just remove the fallback.
Fixes: #7048
Some failures by youtube-dl prompt the user to submit a bug report.
If such a failure occurs, we can compare youtube-dl's version to the
current calendar date to see how old it is. We don't make this check
on every youtube-dl failure, as failing to extract an URL is quite
common, and waiting for a second blocking python interpreter startup
for every such case would be a bit unpleasant.
Here the assumption is made that any youtube-dl version older than
3 months is probably severely out of date. Users will be warned about
this.
We also output the trimmed stderr of youtube-dl with msg.error, as this
appeared to have been the behaviour of utils.subprocess without stderr
capturing. Since this uses mp.command_native now, we'll have to do this
ourselves where appropriate.
mpv warned if the FFmpeg runtime library version was not exactly the
same as the build version. This seemed to cause frequent conflicts. At
this point, most mpv code probably adheres to the FFmpeg ABI rules, and
FFmpeg stopped breaking ABI "accidentally". Another source of problems
were mixed FFmpeg/Libav installations, something which nobody does
anymore. It's not "our" job to check and enforce ABI compatibility
either. So I guess this behavior can be removed.
OK, still check for incompatible libraries (according to FFmpeg
versioning rules), i.e. different major versions, or if the build
version is newer than the runtime version. For now.
The comment about ABI problems is still true. In particular, the
bytes_read field mentioned in the removed comment is still accessed, and
is still an ABI violation. Have fun.
The --cache-pause feature (enabled by default) will pause playback for a
while if network runs out of data. If this is not done, then playback
will go on frame-wise (as packets are slowly read from the network and
then instantly decoded and displayed). This feature is actually useless,
as you won't get nice playback no matter what if network is too slow,
but I guess I still prefer this behavior for some reason.
This commit changes this behavior from using the demuxer cache state
only, to trying to use underrun information from the AO/VO. This means
if you have a very large audio buffer, then cache-pausing will trigger
once that buffer is depleted, which will be some time _after_ the
demuxer cache has run out.
This requires explicit support from the AO. Otherwise, the behavior
should be mostly the same as before this commit.
This does not care about the AO buffer. In theory, the AO may underrun,
then the player will write some data to the AO buffer, then the AO will
recover and play this bit of data, then the player will probably trigger
the cache-pause behavior. The probability of this happening should be
pretty low, so I will hold off fixing this until the next refactor of
the AO chain (if ever).
The VO underflow detection was devised and tested in 5 minutes, and may
not be correct. At least I'm fairly sure that the combination of all the
factors should make incorrect behavior relatively unlikely, but problems
are possible.
Also, the demux_reader_state.underrun field may be inaccurate. It's only
the present state at the time demux_get_reader_state() was called, and
may exclude past underruns. In theory, this could cause "close" cases to
be missed. Then you might get an audio underrun without cache-pausing
acting on it. If the stars align, this could happen multiple times in
the row, effectively making this feature not work.
The most user-visible consequence of this change is that the user
will now see an AO underrun warning every time the cache runs out.
Maybe this cache-pause feature should just be removed...
AOs can now call ao_underrun_event() (in any context) if an underrun has
happened. It will print a message.
This will be used in the following commits. But for now, audio.c only
clears the underrun bit, so that subsequent underruns still print the
warning message.
Since the underrun flag will be used in fragile ways by the playback
state machine, there is the "reports_underruns" field that signals
strong support for underrun reporting. (Otherwise, underrun events will
not be used by it.)
Unless --video-latency-hacks, always decode 2 frames on playback
restart. This in turn will always compute the correct frame duration
(even for the first frame), which in turn happens to fix that playback
with an image at the beginning breaks display.
If a still image precedes video, and the size/format of the frame is
different from that of the video following it, the incorrect frame
duration caused vo_reconfig2() to be called early, causing the window to
resize, and the renderer to clear the image to black. Specifically, it
hit the default value of 1 second duration (for still images), so the
image was displayed for 1 second, and changed to black until the next
proper video frame was displayed.
Normally this does not happen. Even if a video file displays still
images, it normally repeats the still image at the video's FPS (which is
sane). But you can construct such files, or use EDL to construct
something similarly behaving.
This change may increase seek latency a bit in audio video-sync mode
(the default). It needs to wait until 2 frames are decoded, before it
bothers to display the first frame. This is done even when seeking. In
theory it might be good to introduce a "seek preview" mode, which shows
the target image without all the preparations needed for starting
playback. (For example, it could not decode audio.) But since I'm using
video-sync=display-resample, which already needed to always decode 2
frames, I don't think this is a terribly high priority, nor do I
consider the slightly slower seeking a regression.
Fixes: #6765
In this case, gapless will most likely not work. It will result in (very
slight) desync, or (more commonly with small buffer sizes), in an
underflow.
I think it would be legitimate to disable gapless at end of playback
completely if video is enabled at all. But this would need an exception
for cover art mode, so I guess the current solution is OK as well.
The justification for this is the fact that the `video-aspect` property
doesn't work well with `cycle_values` commands that include the value
"-1".
The "video-aspect" property has effectively no change in behavior, but
we may want to make it read-only in the future. I think it's probably
fine to leave as-is, though.
Fixes#6068.
The description of the "playback_only" field in the "subprocess" command
says "you can't start it outside of playback". This did not work
correctly: if the player was started in idle mode in the first place,
the subprocess was allowed to run even with playback_only=yes.
This is a bug, and this change fixes it. Add a test for this to
command-test.lua.
For #7025.
This is realized by dvbin-channel-switch-offset,
which is a numeric offset on the channel initially tuned to.
Since the channel list is kept in the stream alone
depending on detected hardware and chosen card,
and no available backchannel to the player, there's no direct
property which could be switched.
Using input.conf like:
H cycle dvbin-channel-switch-offset up
K cycle dvbin-channel-switch-offset down
Q set dvbin-prog "ZDF HD"
allow fast and reliable channel switching again.
Reinitializes the player as is needed when
tuning to a new DVB channel.
Currently triggered when dvbin-prog is written to,
i.e. when the user explicity switches to a channel by name.
Looks like this didn't actually work. Prefetching will do nothing if
there isn't a thread to "drive" it, and the demuxer thread needs to be
explicitly enabled. (I guess I did the worst possible job in verifying
whether this actually worked when I implemented it. On the other hand,
the user didn't confirm back whether it worked, so who cares.)
Like in the previous commit, bad factoring makes everything worse. It
duplicates logic and implementation of enable_demux_thread(), since the
opener thread cannot access the mpctx->opts field freely. But it's deep
night, so fuck it.
Fixes: c1f1a0845eFixes: #6753
demux_start_prefetch() was called unconditionally in two cases. This is
completely wrong. I'm not sure what part of my brain died off that
something this obviously wrong went in.
The prefetch case is a bit more complicated. It's a different thread, so
you can't access just access mpctx->opts there. So add an explicit field
for this, which is the simplest way to get this done. (Even if it's bad
factoring.)
Fixes: c1f1a0845e
Fixes: 556e204a11
Although this was sort of elegant, it just seems to complicate things
slightly. Originally, the API meant that you cache mp_recorder_sink
yourself (which would avoid the mess of passing an index around), but
that too seems slightly roundabout.
In a later change, I want to change the set of streams passed to
mp_recorder_create(), and then I'd have to keep track of the index for
each stream, which would suck. With this commit, I can just pass the
unambiguous sh_stream to it, and it will be guaranteed to match the
correct stream.
The disadvantages are barely worth discussing. It's a new linear search
per packet, but usually only 2 to 4 streams are active at a time. Also,
in theory a user could want to write 2 streams using the same sh_stream
(same metadata, just writing different packets or so), but in practice
this is never done.
That's just a single one. It used to be more, when FFmpeg still required
using pointless accessors for tons of fields, which historically broke
compatibility with Libav. (I think I wrote the patch to deprecate that
crap and to allow direct access myself.)
There may be more exceptions, but I forgot about them. Another point is
that we don't really trust FFmpeg ABI stability, though.
In spdif mode, there are hacks that try to cut audio on frame boundaries
(blame spdif, which is a hack in itself). The "alignment" is used in a
bunch of places, but --end does not respect it. This leads to some audio
that can't be pushed because the alignment is off (I don't know why, not
do I care), which puts audio into an underrun state forever.
Fix this by discarding unusable extra samples if no new data can be
expected.
Fixes: #6935
When the (float) bitrate is returned, it is implicitely converted to an
int64 value, merely discarding the fractional part.
However the bitrate of a CBR track can vary a bit due to timestamp
precision loss after clock conversion (this can affect MPEG-TS audio
tracks). So a bitrate like 191999.999... results in 191999 when
being returned - instead of 192000.
To fix this, apply proper rounding, as already done for the "old" case.
Hereby refactoring the "old" case to also use `llrint`.
The question came up on how a client would figure out where
screenshot-directory saved its screenshots if it contained
mpv-specific expansions. This command should remedy the situation
by providing a way for the client to ask mpv to do an expansion.
reinit_audio_filters_and_output() can fully shutdown the audio chain on
failure. Specifically, it will deallocate mpctx->ao_chain. The value of
that field was cached in ao_c. The code after the call did not account
that the audio chain can be shutdown, and used the stale ao_c value.
Fixes: #6808
If a file format supports PTS resets, get_current_pos_ratio calculates
the ratio using the stored filepos in the demuxer struct instead of a
standard query of the current time in the stream and its total length.
This seems like a reasonable way to avoid weird PTS values, but in
reality this just causes problems and results in inaccurate ratio
values that can affect other parts of the player (most notably the osc
seekbar).
For libavformat formats, demux->filepos is obtained from the
demux_lavf_fill_buffer function which is called on the next packet. The
problem is that there is a slight delay between packets and in some
cases, this delay can be relatively large. That means the obtained
demuxer->filepos value will be very inaccurate since it obtains the pos
from the end of the upcoming packet and not its actual current position.
This is especially noticeable at the very beginning of playback where
get_current_pos_ratio sometimes returns a value of well over 2% despite
less than a second passing in the stream. Another telltale sign is to
simply watch the osc seekbar as a stream progresses and observe how it
loads in staggered steps as every packet is decoded. In contrast, the
seekbar progresses smoothly on the playback of a format that does not
support PTS resets. The simple solution is to instead use the query of
the current time and length of a stream and calculate the ratio that
way.
get_current_pos_ratio will still fallback on using the byte stream
position if the previous queries fail. However, get_current_time will
be more accurate in the vast majority of cases and should be the
preferred method of calculating the position ratio.
This change adds a version of `mpv_command` that also returns a result.
The main rationale behind this is `mpv_command_node` requires defining
multiple structs before you can even use it, which results in a pretty
painful to use interface just to get the result from a command.
There isn't really a good name for this function, so I'm open to
suggestions on a better name for it.
Replace the "+" with "/". The "+" was supposed to imply that the cache
is the sum of the time (demuxer cache) and the size in bytes (stream
cache). We could not provide something nicer, because we had no idea how
many seconds of media was buffered in the stream cache.
Now the stream cache is done, and both the duration and byte size show
the amount buffered in the demuxer cache. Hopefully "/" is better to
imply this properly. Update the manpage explanations too.
When update_prop() successfully fetched a changed property value, it
set prop->changed to true to indicate the success.
mark_property_changed() also always set
prop->changed to true and additionally prop->need_new_value to true
This is the case since 6ac0ef78
If the observed property changes every frame and then due to timing
the next mark_property_changed() is called before
gen_property_change_event() and therefore directly after update_prop(),
prop->need_new_value was again true and indicated that a new value
has to be retrieved with update_prop(). As a result the event for the
last successful update_prop() was never triggered. This meant that
a property change event were never generated for frame-based properties
only for properties that were observed with MPV_FORMAT_NONE or when the
timing was different and gen_property_change_event() was called after
update_prop().
To fix this, mark_property_change() and update_prop() should not use the
same flag to indicate different things and therefore a new flag for
successfully update a property is introduced. But with the now decoupled property
changed and updated the need_new_value flag is redundant and removed completely.
Fixes#4195
With the stream cache gone, this function had almost no use anymore
(other than opening the stream). Improve this by triggering demuxer
cache readahead.
This enables all streams. At this point we can't know yet what streams
the user's options would select (at least not without great additional
effort). Generally this is what you want, and the stream cache would
have read the same amount of data.
In addition, this will work much better for files that e.g. need to seek
to the end when opening (typically mp4, and mkv files produced by newer
mkvmerge versions).
Remove the deselection call from add_stream_track(). This should be
fine, as streams normally start out as deselected anyway. In the
prefetch case, some code in play_current_file() actually deselects it.
Streams that appear during demuxing are disabled by default, so this
doesn't break this logic either.
Fixes: #6753
render api needs to wait for vo to be destroyed before frees the context.
The purpose of kill_cb is to wake up render api after vo is destroyed,
but uninit did that before kill_cb, so kill_cb tries using the freed
memory. Remove kill_cb to fix the issue as uninit is able to do the
work.
wm4
Jan Janssen
Stephan Hilb
Nicolas F
ckath
Niklas Haas
Oliver Freyermuth
Stefan Pöschel
Stefano Pigozzi
Dudemanguy911
wnoun
thewisenerd
Ricardo Constantino
Paul B Mahol
Dark
Gunnar Marten