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
Third try is the charm? I stupidly missed that this option already
existed in my previous commits. Instead, add an auto value to it and
enable it by default for sd_lavc but not sd_ass. On my limited samples,
it seems to fix the gaps issue that can occur but without regressing
some duration timings for sub_lavc subtitles. Well hopefully anyway.
Fixes#12327.
Yet another bad idea. It turns out that there's already a sub-fix-timing
option which logic for this exact thing (overlaps/gaps) fixing. Also it
works better than this since it doesn't appear to artifically increase
sub duration either. Fixes#12327.
This reverts commit 725b631ec3.
The old name is pretty bad and users mistakenly think it has something
to do with selecting forced subtitles (that would be
--subs-fallback-forced). Instead of giving it such a generic name, make
it clearer that this has to do specifically with forced sub events
which is only relevant for a small minority of subtitles.
First of all, this never worked. Or if it ever did, it was in some
select few scenarios. c9474dc9ed is what
originally added support for the auto choice. However, that commit
worked by propagating a value to a fake option used internally. This
shouldn't have ever worked because the underlying m_config_cache was
never updated so the value shouldn't have been preserved when accessed
in sd_lavc. And indeed with some testing, the value there is always 0
unsurprisingly.
This was later rewritten in ba7cc07106
along with a lot of other sub changes, but with that, it was still
mostly broken. The reason is because one of the key parts of having to
hit this logic (prefer_forced) required `--no-subs-with-matching-audio`
to be set. If the audio language matches the subtitle language (the
requirement also excludes forced subs), the option makes no subtitle
selection in the first place so pick->forced_only_def is not set to true
and nothing even happens. Another way around this would be to attempt to
change your OS language (like with the LANG environment variable) so
that the subtitle track gets selected but then audio_matches mistakenly
becomes false because it compares the OS language to the audio language
which then make preferred_forced 0, so nothing happens. I don't think
there's a scenario where pick->forced_only_def is actually set to true
(thus meaning `auto` is useless), but maybe someone could contrive
something very strange. Regardless, it's definitely not something even
remotely common.
fbe8f99194 changed track selection again
but didn't consider this particular case. The net result is that DVD/PGS
subs become equivalent to --sub-forced-only being yes, so this a change
in behavior and probably not a good one. Note that I wasn't able to
actually observe any difference in a PGS sample. It still displayed
subtitles fine but that sample probably didn't have the right flags to
hit the sub-forced-only logic.
Anyways, the auto feature is extremely questionable at best and in my
view, not actually worth it. It is meant to be used with
`--no-subs-with-matching-audio` to display forced pictures in subtitle
tracks that are not marked as forced, but that contradicts that
particular option's purpose and description in the manual (secretly
selecting a track under certain conditions even though it says not to).
Instead of trying to shove all this logic into select_default_track
which is already insanely complicated as it is, recognize that this is a
trivial lua script. If you absolutely want to turn --sub-forced-only on
under these certain conditions (DVD/PGS subtitles, matching audio and
subtitle languages, etc.), just look at the current-tracks property and
do your thing. The very, very niche behavior that this option tried to
accomplish basically never worked, no user even knows what this option
does, and well it's just not worth supporting in core mpv code. Drop
all this code for sanity's sake and change --sub-forced-only back to a
bool.
It turns out this already exists for sd_ass and is being used there. We
can make use of this arbitrary threshold instead for overlapping
subtitle durations to avoid the weird flashing behavior with some
pgs subtitles.
mpv makes this option an integer, but the underlying ass API actually
accepts doubles. From some testing, there is no meaningful precision
difference between float or double (it seems to go in roughly 0.05
steps), so just make it a float. sd_lavc also can handle non-integer
values here. Closes#11583.
When getting subtitles, sd_lavc checks if the current pts plus a small
offset (1e-6) is greater than the sub->pts as well as checking if the
pts is less than the sub->endpts. The problem with the endpts check is
that there are subtitles out there (pgs ones) that have overlapping
durations and thus you'll get a case where pts is greater than endpts
because a new subtitle shows up. However, the old subtitle is still
meant to be on the screen. This results in a flickering effect where the
subtitle flashes and then appears the next frame. The easy enough fix is
to just loosen the condition and remove the endpts check altogether.
That ensures that the subtitle remains selected for the entire duration.
Unsure if this possibly could regress some other kind of subtitle out
that that uses sd_lavc, but this does appear to fix a real issue with
pgs subtitles. Fixes#8202 and #10051.
Internal subtitles were not shown when switching between tracks while
mpv was paused. The reason for this is simply because the demuxer data
isn't available yet when the track switch happens. Fixing it is
basically just retrying until the packet is actually available when the
player is paused. Fixes#8311.
This only existed as essentially a workaround for meson's behavior and
to maintain compatibility with the waf build. Since waf put everything
in a generated subdirectory, we had to put make a subdirectory called
"generated" in the source for meson so stuff could go to the right
place. Well now we don't need to do that anymore. Move the meson.build
files around so they go in the appropriate place in the subdirectory of
the source tree and change the paths of the headers accordingly. A
couple of important things to note.
1. mpv.com now gets made in build/player/mpv.com (necessary because of
a meson limitation)
2. The macos icon generation path is shortened to
TOOLS/osxbundle/icon.icns.inc.
This is yet another unfortunate side effect of the width % SLICE_W == 0
special case. While looping through these rectangles, the rc->x1 value
on the final loop can be greater than the actual total width. This will
cause a buffer overflow if using the mp_draw_sub_overlay API. 2 of the
current VOs that use that work around it by adjusting the values
returned, but the better fix is to correct what's actually given in the
rectangles so you can safely use the values. As for the fix, it's simply
ensuring that rc->x1 doesn't extend beyond p->w with a MPCLAMP.
Previously, the code would always naively add SLICE_W (256) to rc->x0
which would easily extend past the actual width in many cases. The
adjustments in vo_vaapi and vo_dmabuf_wayland are dropped and in theory
vo_direct3d should work now since we can just trust the values given to
us in the rectangles. How nice.
Libass commit f08f8ea5 (between 0.16 and 0.17) changed how PlayResX
affects some aspects of rendering.
The libass change fixes a VSFilter compatibility issue which existed
for about two decades, and there are no libass plans to support the
previous behavior, so ultimately we have to adjust the mpv code, and
we can't guarantee to restore the old behavior in all cases.
Starting at this commit, vector drawing coords, font spacing, border
and shadow widths are all affected by PlayResX (specifically, by
the aspect), while previously they were unaffected by PlayResX.
This changed converted sub border and shadow widths in mpv, because
ffmpeg generates the ass with fixed PlayResX of 384 (aspect of 4:3),
and with libass 0.17, if this doesn't match the display aspect, then
borders and shadow were too wide - because most clips aspect is more
than 4:3.
The fact that ffmpeg uses fixed PlayResX of 384 could be considered
an issue, but for now we have no control over it, and ffmpeg doesn't
have the video resolution when it converts an srt source to ass.
So here we adjust PlayResX accordingly so that border/shadows are
now rendered with correct width.
However, regardless of that commit, changing PlayResX also affects
the margin value, so to compensate, we adjust sub-margins-x too.
According to libass devs, this should cover basic srt-to-ass
conversion by ffmpeg to work correctly with libass 0.17.
However, there could be srt extensions which use more complex ass,
and/or ffmpeg conversion of other sub formats (such as aribb24,
aribcaption and movtext), where more things need adjustments.
As of now we don't know what these are, and so we don't really know
what else might remain broken or get broken.
dbc5d7b7db seems to have originally
introduced this behavior. At the time, wm4 simply reconfigured ass on
every frame in order to accommodate runtime changes in sub options. This
certainly works, but these libass API calls are not free and there is at
least one known performance regression due to a change in libass*.
Regardless of whether or not the libass change is good/bad, there is no
need for mpv to constantly reconfigure this. When wm4 made that commit,
there was no notification mechanism for options changing that could
easily be used so he didn't really have any other choice. But it's
nearly 10 years later now and internally we have all the necessary
pieces to only configure ass again when we need to: on option changes or
resizes. So go ahead and implement that in this commit which simply uses
the already existing SD_CTRL_UPDATE_OPTS and compares osd_res sizes to
determine whether or not an ass configure is needed.
*: https://github.com/libass/libass/issues/698
Essentially the same as d1d2370d07 except
for clear_rgba_overlay. From some empirical testing, the s->x1 value is
either 0 or SLICE_W (256). In the case where it is 256 and s->x0 is 0,
then it writes memory. For most slices, this is fine. However for the
very last slice in the loop, it is possible for the width here to exceed
the total width of the line (p->w). If that occurs, the memset triggers
a buffer overflow. This last case needs to be guarded in the same way as
the previously mentioned commit (total width - SLICE_W * x) and with
MPMIN in case s->x1 is 0 here to preserve the old behavior. It's unknown
if anything other than dmabuf-wayland can possibly hit this, but it's
definitely a problem within mp_draw_sub_overlay itself.
this changes mp_image_new_ref() to handle allocation failure itself
instead of doing it at its many call-sites (some of which never checked
for failure at all).
also remove MP_HANDLE_OOM() from the call sites since this is not
necessary anymore.
not all the call-sites have been touched, since some of the caller might
be relying on `mp_image_new_ref(NULL)` returning NULL.
Fixes: https://github.com/mpv-player/mpv/issues/11840
Upon an option update with an UPDATE_SUB_HARD flag,
the ass_track that stores all the decoded
subtitle packets/events is destroyed and recreated, which means
the packets need to be read and decoded again to refill
the ass_track. This caused issues (no subs displayed) in 2 cases:
1. external sub files
Previously, external sub files were read and decoded only
once when loaded. Since this meant all decoded events were lost
forever when recreating the ass_track, we need to change this
and trigger a new preload during sub reinits.
2. converted subs (non-ASS text subs like srt)
For converted subs, we maintain a list of previously
seen packets to avoid decoding and adding duplicate events
to the ass_track. Previously this list wasn’t synchronized with
the corresponding ass_track, so the sub decoder would reject
any previously seen sub packets, usually meaning only subs sometime
after the current pts would be displayed after sub reinits.
Fix this by resetting the list upon ass_track recreation.
This way we receive such minor details as the profile (necessary for
ARIB captions, among others) during init. This enables decoders
to switch between ARIB caption profile A and profile C streams.
This reworks all of mpv's unit tests so they are compiled as separate
executables (optional) and run via meson test. Because most of the tests
are dependant on mpv's internals, existing compiled objects are
leveraged to create static libs and used when necessary. As an aside, a
function was moved into video/out/gpu/utils for sanity's sake (otherwise
most of vo would have been needed). As a plus, meson multithreads
running tests automatically and also the output no longer pollutes the
source directory. There are tests that can break due to ffmpeg changes,
so they require a specific minimum libavutil version to be built.
These options make it possible to specify the directory that will be
passed to ass_set_fonts_dir(), akin to VLC's `--ssa-fontsdir` and
FFmpeg's `fontsdir`.
Fixes#8338
c784820454 introduced a bool option type
as a replacement for the flag type, but didn't actually transition and
remove the flag type because it would have been too much mundane work.
The `osd-overlay` command didn't trigger a redraw when the overlay was
set to hidden.
This is fine when the overlay was already hidden before that, but
transitioning from not hidden to hidden requires a redraw.
e97819f88e corrected a special case
condition that lead to an out of bounds access if the total width
happened to be an integer multiple of SLICE_W (256) which could cause a
crash in software VOs. However, it turns out that the functions in this
file evaluate quite differently when using encoding mode (and presumably
libmpv as well according to reports although I could not independently
verify it).
The logic here gets complicated but what ends up happening is that, in
blend_overlay_with_video, the value of x + w can be greater than p->w in
certain cases in encoding mode. The x is the positional value of the
slice which remained unchanged from before, but w can take the full
value of SLICE_W (256) which is not necessarily correct. The width of
the final slice here should be the total remaining width. We can handle
this in mark_rect by simply always adjusting x1 of the last slice to be
equal to total width - SLICE_W * x so it can never extend beyond where
it should be. In practice, this value should be the maximum allowed
here. I'm not sure if the existing x1 value can possibly already be
lower than SLICE_W, but just MPMIN it to be on the safe side.
Fixes#10908.
This has been a long standing annoyance - ffmpeg is removing
sizeof(AVPacket) from the API which means you cannot stack-allocate
AVPacket anymore. However, that is something we take advantage of
because we use short-lived AVPackets to bridge from native mpv packets
in our main decoding paths.
We don't think that switching these to `av_packet_alloc` is desirable,
given the cost of heap allocation, so this change takes a different
approach - allocating a single packet in the relevant context and
reusing it over and over.
That's fairly straight-forward, with the main caveat being that
re-initialising the packet is unintuitive. There is no function that
does exactly what we need (what `av_init_packet` did). The closest is
`av_packet_unref`, which additionally frees buffers and side-data.
However, we don't copy those things - we just assign them in from our
own packet, so we have to explicitly clear the pointers before calling
`av_packet_unref`. But at least we can make a wrapper function for
that.
The weirdest part of the change is the handling of the vtt subtitle
conversion. This requires two packets, so I had to pre-allocate two in
the context struct. That sounds excessive, but if allocating the
primary packet is too expensive, then allocating the secondary one for
vtt subtitles must also be too expensive.
This change is not conditional as heap allocated AVPackets were
available for years and years before the deprecation.
It turns out, even xy-VSFilter and XySubFilter do not
mangle colours if the video is native RGB regardless of
the sub's YCbCr header. libass' docs were also updated
to reflect this.
Commit 740b701 introduced handling for subtitles with unknown duration,
but it came with a minor flaw where a track event that shares identical
start time with following track event will has its Duration value set
to 0, we don't want this to happen since it will prevent simultaneous
rendering of multiple track events.
This commit aims to address this issue.
When the width is exactly a multiple of SLICE_W (currently 256),
heap buffer overflow is reported by Address Sanitizer. So adjust
the maximum index for the line array accordingly.
ASS must only automatically break at ASCII spaces (\x20), but other
subtitle formats might expect more breaking oppurtinities.
Especially non-ASS subs in scripts, which typically do not use (ASCII)
spaces to seperate words, like e.g. CJK, might overflow the screen
if the conversion didn't insert additional linebreaks (ffmpeg does not).
Thus try to enable Unicode linebreaking for converted subs and the OSD
if libass is new enough. The feature may still be unavailable at runtime
if libass wasn't build with Unicode linebreaking support.
TL;DR: previously a JavaScript VM was created + destroyed whenever
a sub track was initialized, even if no jsre filter was set.
Now a JS VM is created only if jsre filters were set.
Sub filters are initialized once when a subtitle track is chosen, and
then whenever the sub track changes or when some sub options change.
Sub filters init is synchronous - playback is suspended till it ends.
A filter can abort init early (get disabled) depending on conditions
specific to each filter. The regex and jsre filters aborted early
if the filter is disabled (default is enabled) or if the track is not
ass (relativey rare, e.g. bitmap subs).
The init then iterates over the filter strings, and if the result is
empty (common - no filter was added, but also if all strings failed
regex init) then it's also aborted during init.
While this iteration step is cheap with filter regex, with jsre it
requires instanciating the JS VM (mujs) in advance in order to parse
the filter strings at the list, and the VM is then destroyed if the
list ends up empty.
This VM create+destroy is fast but measurable (0.2 - 0.7 ms, slowest
measured on 2010 MacBook Air), but can be avoided altogether if we
check that the filter list is not empty before we create the VM.
So now we do just that.
this field is used only in a special vo draining edge case.
switching to an atomic reduces osd->lock contention
between the mpv core (in write_video) and vo threads
which are managing osd rendering manually (such as vo_rpi).
Signed-off-by: Aman Karmani <aman@tmm1.net>
Previously, the sub-visibility option changed the visibility of all
subtitles including secondary ones. This meant that it was not possible
to only display secondary subtitles while hiding the primary ones. This
modifies the sub-visibility option so that it only affects primary
subtitles which allows only secondary subtitles to be displayed.
This reverts commit 04f0b0abe4.
It's not a good idea to unify the names only for visibility, while
keeping secondary-* for everything else.
This needs a bit more thought before we allow secondary sub to be
visible on its own.
Adds --sub-visibility choices 'primary-only' for only displaying the
primary subtitle track, and 'secondary-only' for only displaying
secondary subtitle track.
Removes --secondary-sub-visibility and displays a message telling the
user to use --sub-visibility=yes/primary-only instead.
These changes make it so that the default 'sub-visibility' bind 'v'
cycles through all the 'sub-visibility' choices, 'no', 'yes',
'primary-only', and 'secondary-only'.
Since libavcodec major version 59, the requested "ass" format became
the default as the old timing-included format was disabled starting
with that version. Additionally, this option by itself has since
been deprecated as it no longer serves any purpose.
References:
FFmpeg/FFmpeg@1f63665ca5FFmpeg/FFmpeg@176b8d785bFixes#9413
This is an artificial background box with the original colors,
rendered as a plain rectangle instead of libass's opaque-box.
Fixes#9134 (together with the previous commit)
This breaks the rendering in various ways, so first workaround is to
simply disable the opaque box for the bar.
Next commit will add an artificial background box.
llyyr
Kacper Michajłow
Dudemanguy
NRK
rcombs
Lypheo
Alexander Seiler
Jan Ekström
Frédéric Brière
Christoph Heinrich
Thomas Weißschuh
Philip Langdale
Oneric
VincentVerdynanta
Shreesh Adiga
Avi Halachmi (:avih)
Guido Cella
Cœur
Aman Karmani
Ripose
Niklas Haas