sd_lavc: mitigate evil rounding issue that could lead to off-by-1 frames

A mkv sample file was provided to me, which contained a moving PGS
subtitle track, with the same track rendered into the video as
reference. The subtitle track appeared to stutter (while the video one
was smooth). It turns out this was a timestamp rounding issue in mpv.

The subtitle timestamps in the file match the video ones exactly.
They're the same within the mpv demuxer too. Unfortunately, the
conversion from and to libavcodec timestamps is lossy, because mpv uses
a non-integer timebase, while libavcodec supports integers only. See
mp_pts_to_av() and mp_pts_from_av(). The recovered timestamp is almost
the same, but is off by a very minor part. As a result, the timestamps
won't compare equal, and if that happens, display of the subtitle frame
is skipped. Subtitle timestamps don't go through this conversion
because... libavcodec is special? The libavcodec subtitle API is
special.

Fix this by giving it a microsecond of slack. This is basically as if we
used an internal microseconds integer timebase, but only for the purpose
of image subtitle display.

The same could happen to sd_ass, except in practice it doesn't. ASS
subtitles (well, .ass files) inherently use a timebase incompatible to
video, so to ensure frame exactness, ASS timestamps are usually set to
slightly before the video frame's.

Discussion of better solutions:

One could rewrite mpv not to use float timestamps. You'd probably pick
some integer timebase instead (like microseconds), which would avoid the
libavcodec interop issue. At the very least this would be a lot of work.

It would be interesting to know whether the rounding in ther mpv<->lavc
timestamp conversion could be fixed to round-trip in this case. The
conversion tries to avoid problems by using the source timebase (e.g.
milliseconds with mkv). But in general some rounding is unavoidable,
because something between decoder and lowest demuxer layer could
transform the timestamps.

One could extend libavcodec to attach arbitrary information to avpacket
and return it in the resulting avframe. To some degree, such a mechanism
already exists (side data). But there are certain problems that make
this unfeasible and broken.

One could pass through exact mpv float timestamps by reinterpret-casting
them to int64_t, the FFmpeg timestamp type. Actually mpv used to do
this. But there were problems, such as FFmpeg (or things used by FFmpeg)
wanting to interpret the timestamps. Awful shit that make mpv change to
the current approach.

There's probably more but I'm getting bored. With some luck I wasted
precious seconds of your life with my nonsense.
This commit is contained in:
wm4 2020-04-18 00:10:34 +02:00
parent 34e7d9c2f4
commit ab201ce042
1 changed files with 1 additions and 1 deletions

View File

@ -387,7 +387,7 @@ static struct sub *get_current(struct sd_lavc_priv *priv, double pts)
if (!sub->valid)
continue;
if (pts == MP_NOPTS_VALUE ||
((sub->pts == MP_NOPTS_VALUE || pts >= sub->pts) &&
((sub->pts == MP_NOPTS_VALUE || pts + 1e-6 >= sub->pts) &&
(sub->endpts == MP_NOPTS_VALUE || pts < sub->endpts)))
{
// Ignore "trailing" subtitles with unknown length after 1 minute.