0b35b4c917 originally introduced sd_filter
to make a more general subtitle filter infrastructure. But when doing
so, it directly sets ft->event_format to ass_track->event_format in the
struct. The lifetime of ass_track and the sd_filter are not equivalent
which makes it easy to trigger undefined behavior. Notably, commit
cda8f1613f introduced assobjects_destroy
which can destroy ass_track anytime during runtime which means that the
string in ft->event_format is actually freed and should never be used.
Remedy this by simply doing a proper strdup when the filter inits with
ft as the parent so we avoid this scenario altogether. Fixex #13525.
An ass event like `{\p1\pos{1,1}}m 0 0 l -3 -7 l 11 -7 l 11 -2` ends
the drawing mode started with `\p1` due to `\pos` gets confused with
`\p`, and thus that line is wrongly considered to be visible text.
LRC subtitles can have lines with multiple timestamps, e.g.
[00:00.00][00:02.00]foo
[00:01.00]bar
Currently mpv shows only the "foo" that was decoded first, because it
compares the packet file position to check if a packet was already seen,
and it is the same for both occurrences of "foo". Fix this by also
comparing the pts.
This keeps comparing the packet position on top of the pts to not break
subtitle lines with the same timestamp, like:
1
00:00:00,000 --> 00:00:01,000
foo
2
00:00:00,000 --> 00:00:01,000
bar
where mpv shows both lines on top of each other. They are common in ASS
subtitles.
Fixes https://github.com/mpv-player/mpv/issues/13497.
When using sub-seek without a video track while paused, adding the 0.01
SUB_SEEK_OFFSET to the new timestamp is not enough to show the new
subtitle line. Add 0.1 instead to fix it. 0.01 is already enough for
sub-step.
The previous commits optimized sub redrawing on still images/terminal so
mpv wouldn't redraw so much. There is a gap though. It only assumes
static subtitles. Since ASS can be animated, those types of subtitles
will always need redraws so we need to build in specific detection for
this. We need to build a whitelist of events in ASS that are considered
animations and then flag the packet. Additionally, there's a bunch of
annoying bookkeeping that has to be done since packets can be dropped on
seeks and so on.
This only affects two special cases: printing subtitles to the terminal
and printing subtitles on a still picture. Previously, mpv was very dumb
here and spammed this logic on every single loop. For terminal
subtitles, this isn't as big of a deal, but for the image case this is
pretty bad. The entire VO constantly redrew even when there was no need
to which can be very expensive depending on user settings.
Instead, let's rework sub_read_packets so that it also tells us whether
or not the subtitle packets update in some way in addition to telling us
whether or not to read more. Since we cache all packets thanks to the
previous commit, we can leverage this information to make a guess
whether or not the current subtitle packet is supposed to be visible on
the screen. Because the redraw now only happens when it is needed, the
mp_set_timeout_hack can be removed.
Previously, mpv only saved a strict limit of two packets while decoding
subtitles, but it is a bit incomplete. Firstly, it's certainly possible
for there to be more than two subtitles visible at once. And also, it
did not take into account preloading. Rework this mechanism so that it
is a growable array that can store as many packets as we want. Note that
in this commit, the packets are only ever discarded on reset or destroy,
so in theory it could grow forever. Some discarding logic will be added
in the next commit since it is inherently tied to other things.
* Range of accepted values for teletext_page now include 0 and -1.
* 0 means "subtitle" and -1 means "*".
* Make 0 the default.
Signed-off-by: Mohammad AlSaleh <CE.Mohammad.AlSaleh@gmail.com>
Before 3250f6e447, secondary subtitles
didn't work properly with sd_lavc at all. They would render but be in
the same position as the primary subtitles. sd_ass used SD_CTRL_SET_TOP
to specifically place secondary subtitles in a different spot, but it
was unused by sd_lavc. With that above mentioned commit, it became
possible to distinguish between the position of primary and secondary
subtitles via mpv's option system. sd_lavc was missed however because at
the time we simply didn't realize this could work and was actually
simple. With some subsequent refactoring that happened later, the fix is
to just use the correct sd->order when accessing the shared subtitle
options instead of hard coding 0. Fixes#13440.
f9cefbfec4 made it so mp_ass_flush_old_events() is continously called on
subtitles with unknown duration, without explaining why, breaking
sub-seek/step -1 with a VO (the issue does not occur when showing
subtitles in the terminal because get_bitmaps() is not called). I don't
experience any issue after removing the call, so delete it to fix these
commands.
After removing that, you can sub-seek -1 once after regular playback,
but not after seeking and thus not multiple times in a row. This is
caused by a714f8e928 which fixed subtitles with unknown duration being
duplicated when seeking with a VO (it does not happen in the terminal)
by clearing old lines on seeks, which broke sub-seek -1 and sub-step -1
in a second way after any seek. The proper fix is to remove the line
ctx->num_seen_packets = 0 for subtitles with unknown duration instead,
which lets decode() return early when a line has already been shown.
Having removed these 2 lines, I also removed sd->preload_ok = false, and
thus the whole conditional, since according to sub/sd.h preload_ok only
needs to be set to false when old subtitles are discarded, and they are
no longer discarded,
The bug can be reproduced with
mpv --sub-file=<(curl 'https://music.xianqiao.wang/neteaseapiv2/lyric?id=1399616170' | jq -r .lrc.lyric) 'https://www.youtube.com/watch?v=dQw4w9WgXcQ'
strlen is only relevant if the length is less than [1, 4], so this can
be replaced with strnlen instead which will only traverse characters
upto the maxlen insted of the entire string length. It also makes MPMIN
unneeded. Also fix a comment.
The +1 here is not correct. For a 4-byte unicode character, this would
throw a runtime error because the +1 would try to assign the null
terminator past the actual bound of our array. Just remove it since it
should be exactly equal to whatever we have for bytes.
There's a simple check in filter_sdh that gets the bytes of the first
character in a string in order to do pointer arthimetic to filter the
string. The problem is that it is possible for the amount of bytes to be
greater than the actual length of the string for certain unicode
characters. This can't be worked with so enforce the strlen as the
absolute minimum here to avoid overflow situations.
Fixes#13237.
Missed in 3250f6e447. Note that the
hardcoded ass alignment value is not used anymore as of that commit, but
we should ideally be moving towards secondary subs actually being
customizable via ASS anyways.
Over the years, we've accumulated several secondary subtitle related
options and properties, but the implementation was not really consistent
and it wasn't clear what the right process for adding more should be. So
to make things nicer, let's refactor all of the subtitle options with
secondary variants (sub-delay, sub-pos, and sub-visibility) and split
them off to a new, separate struct. All of the underlying values are
stored in an array instead for simplicity. Additionally, the
implementation of some secondary-sub-* properties were slightly changed
so there would be less redundancy.
This filter is a bit complicated, but one of the essential parts of it
is removing text enclosed by particular set of characters (e.g. text
inbetween []). This was previously hardcoded to only take into account
parenthesis and brackets, but people may want to filter more things so
make this customizable. The option only takes "left hand characters" so
the right pair is mapped internally if applicable. If not, then we just
use the same character. Fixes#8268 since the unicode character in
question can just be passed to this option.
These two functions are almost exactly the same. The parenthesis variant
is essentially just a special case with more conditions to not remove
text. These can easily be combined together into one generic
skip_enclosed function to handle both cases. We also use char * instead
of char for the character comparison here since not everything is
neccesarily 1 byte and can fit into a char. This will be useful for the
following commits where we extend this logic further.
Add --secondary-sub-delay option and decouple --sub-delay from secondary
subtitles. This produces desirable behavior in most cases as secondary
and primary subtitles tracks tend to be timed independently of one
another.
This feature is implemented by turning the sub_delay field in
mp_subtitle_opts into an array of 2 floats. From here the track index is
either passed around or derived when sub_delay is needed. There are some
cases in dec_sub.c where it is possible for dec_sub.order (equivalent to
track index) to be -1. In these cases, sub_delay is inferred as 0.
We prefer to fail fast rather than degrade in unpredictable ways.
The example in sub/ is particularly egregious because the code just
skips the work it's meant to do when an allocation fails.
Stretch a subtitle duration so it ends when the next one starts.
Should help with subtitles which erroneously have zero durations.
I found such a subrip substitles stream in the wild.
Signed-off-by: Mohammad AlSaleh <CE.Mohammad.AlSaleh@gmail.com>
In the sub seek code path, there was an arbitrary small offset added to
the pts before the seek. However when seeking backwards, the offset was
an additional subtraction. de6eace6e9
added this logic 10 years ago and perhaps it made sense then, but the
additional subtraction when seeking backwards causes the subtitle seek
to go too far to the previous subtitle if the durations overlap. This
should always be an addition to work correctly. Additionally, the sub
stepping code path also could use this offset for the same reason
(duration overlaps). However, it is only applicable to sd_ass not
sd_lavc. sd_lavc has step_sub support but on a sample it didn't even
work anyway. Perhaps it only works for certain kinds of subtitles
(patches welcome).
Anyways instead of keeping this offset as a magic number, we can define
it in sd.h which is handy for this. For sd_ass, we add the offset when
sub stepping, and the offset is always added for sub seeking like it was
before. Update the comment to be a little more relevant to what actually
happens today. Fixes#11445.
Since 062104d16e, we started saving cached
packets for subtitles. However, these can point to the same address as
what is stored in sub->new_segment. When a segment is updated, the
packet is potentially freed. Later during decoding, this can lead to a
double free since the cached packets will naturally try to free itself
and update. Fix this by simply always making sub->new_segment a full
copy of the packet so its lifetime doesn't have to be tied to the cached
packet stuff.
UPDATE_SUB_HARD causes all of the ass objects to reset in order to apply
the new style. UPDATE_SUB_FILT doesn't actually reset the sd, but it
should in order to update the actual filters so that was added here.
Doing this causes the current subtitle to be dropped. In the paused
cause, this concidentally works because command.c forces a video refresh
which then reloads the subtitle essentially. But while playing, the
subtitle will be dropped and you won't get anything until the next one
appears.
Instead of using video refreshes, what we can do is just always save the
last two subtitle packets in a cache and redecode them if needed. This
is much easier and also allows us to get rid of all the video refresh
logic in command.c. Fixes#12386.
This option has exactly the same semantics are other mpv options that
override a particular thing with something from the user. So instead of
the "force-style" name, use "-overrides" which is more consistent.
The plural form is used since it's a list option.
since i was going to fix the include order of stdatomic, might as well
sort the surrouding includes in accordance with the project's coding
style.
some headers can sometime require specific include order. standard
library headers usually don't. but mpv might "hack into" the standard
headers (e.g pthreads) so that complicates things a bit more.
hopefully nothing breaks. if it does, the style guide is to blame.
replace it with <stdatomic.h> and replace the mp_atomic_* typedefs with
explicit _Atomic qualified types.
also add missing config.h includes on some files.
The function called to get sub-start and sub-end returns early when the
subtitle's duration is unknown, but by just removing this check the
properties work fine. The final sub line has a very large sub-end, but
that is much better than not having the properties work at all.
The stream selection state wasn't improved. I didn't realize this messed
with caches. All in all, just not a good idea. Back to drawing board I
guess.
This reverts commit f40bbfec4f.
This replaces the previous commit and makes more sense. The internal
demux marked tracks as eager depending on their type and for subtitles
it would always lazily read them unless there happened to be no
available av stream. However, we want the sub stream to be eager if the
player is paused. The existing subtitle is still preserved on the
screen, but if the user changes tracks that's when the problem occurs.
So to handle this case, propagate the mpctx->paused down to the stream
selection logic. This modifies both demuxer_refresh_track and
demuxer_select_track to take that boolean value. A few other parts of
the player use this, but we can just assume false there (no change in
behavior from before) since they should never be related to subtitles.
The core player code is aware of its own state naturally, and can always
pass the appropriate value so go ahead and do so. When we change the
pause state, a refresh seek is done on all existing subtitle tracks to
make sure their eager state is the appropriate value (i.e. so it's not
still set to eager after a pause and a track switch). Slightly invasive
change, but it works with the existing logic instead of going around it
so ultimately it should be a better approach. We can additionally remove
the old force boolean from sub_read_packets since it is no longer
needed.
Actually, I thought of a better way of handling this shortly after
merging this. Revert it and redo it in the next commit.
This reverts commit c2c157ebec.
a323dfae42 almost fixed subtitle tracks
disappearing when paused but it actually missed one part: the behavior
of demux_read_packet_async_until. It's a bit unintuitive, but for
subtitle streams, that function would only return the very first packet
regardless of whatever pts you pass to it. So the previous commit worked
on the very first subtitle, but not actually any of the others (oops).
This is because subtitle streams never marked as eager and thus never
actually read farther ahead. While the video is playing, this is OK, but
if we're paused and switching subtitle tracks then the stream should be
eagerly read. Luckily, the logic is already there in the function for
this. All we have to do add an extra argument to
demux_read_packet_async_until to force the stream to be read eagerly and
then it just works. Be sure to unset the eager flag when we're done.
Actually fixes the bug for real this time.
Packet duration is not necessarily related to the display time of the
subtitle. Use start/end_display_time fields as source of the timing.
Fixes subtitles with infinite duration that should be on screen until
next sub is displayed.
Timestamps are converted from microsecond resolution timestamp, we don't
have more precision, so we have to account for that when comparing the
floating point values as them will slightly be off.
Fixes: #12327
Kacper Michajłow
Dudemanguy
Christoph Heinrich
Guido Cella
Mohammad AlSaleh
dyphire
karelrooted
Ripose
sfan5
NRK
llyyr