RepoMirrors/mpv
1
0
Fork 0
mirror of https://github.com/mpv-player/mpv synced 2024-12-26 00:42:57 +00:00
Code Issues Releases Wiki Activity

demux: another questionable backwards playback mud party

Browse Source 
In theory, backward demuxing does way too much work by doing a full
cache seek every time you need to step back through a packet queue. In
theory, it would be exceedingly more efficient to just iterate backwards
through the queue, but this is not possible because I'm too stingy to
add 8 bytes per packet for backlinks. (In theory, you could probably
come up with some sort of deque, that'd allow efficient iteration into
any direction, but other requirements make this tricky, and I'm
currently too dumb/lazy to do this. For example, the queue can grow to
millions of elements, all while packet pointers need to stay valid.)

Another possibility is to "locally" seek the queue. This still has less
overhead than a full seek.

Also, it just so happens that, as a side effect, this avoids performing
range merging, which commit f4b0e7e942 "broke". That wasn't a bug at
all, but since range joining is relatively slow, avoiding it is good.
This is really just a coincidental side effect, I'm not even quite sure
why it happens this way.

There are 4 ugly things about this change:

1. To get a keyframe "before" a certain one, we recompute the target
PTS, and then subtract 0.001 as arbitrary number to "fudge" it. This
isn't the first place where this is done, and hey, it wasn't my damn
idea that MPlayer should use floats for timestamps. (At first, it even
used 32 bit timestamps.)

2. This is the first time reader_head is reset to an earlier position
outside of the seek code. This might cause conceptual problems since
this code is now "duplicated".

3. In theory, find_backward_restart_pos() needs to be run again after
the backstep. Normally, the seek code calls it explicitly. We could call
it right in the new code, but then the damn function would be recursive.
We could shuffle the code around to make it a loop, but even then
there'd be an offchance into running into an unexpected corner case (aka
subtle bug), where it would loop forever. To avoid refactoring the code
and having to think too hard about it, make it deferred - add some new
state and the check_backward_seek() function for this. Great, even more
subtle mutable state for this backwards shit.

4. I forgot this one, but I can assure you, it's bad.

Without doubt someone will have to clean up this slightly in the future
(or rip it out), but it won't be me.
This commit is contained in:
wm4 2019-06-16 17:44:27 +02:00
parent 2f64c84b44
commit f2cee22311
1 changed files with 41 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
demux/demux.c
View File

@ -211,8 +211,9 @@ struct demux_internal {
// stuff otherwise), which adds complexity on top of it.
bool back_demuxing;
// For backward demuxing: back-step seek needs to be triggered.
bool need_back_seek;
// For backward demuxing:
bool need_back_seek; // back-step seek needs to be triggered
bool back_any_need_recheck; // at least 1 ds->back_need_recheck set
bool tracks_switched; // thread needs to inform demuxer of this
@ -379,6 +380,7 @@ struct demux_stream {
double last_ret_dts;
// Backwards demuxing.
bool back_need_recheck; // flag for incremental find_backward_restart_pos work
// pos/dts of the previous keyframe packet returned; always valid if back-
// demuxing is enabled, and back_restart_eof/back_restart_next are false.
int64_t back_restart_pos;
@ -419,6 +421,9 @@ static bool queue_seek(struct demux_internal *in, double seek_pts, int flags,
static struct demux_packet *compute_keyframe_times(struct demux_packet *pkt,
double *out_kf_min,
double *out_kf_max);
static void find_backward_restart_pos(struct demux_stream *ds);
static struct demux_packet *find_seek_target(struct demux_queue *queue,
double pts, int flags);
static uint64_t get_foward_buffered_bytes(struct demux_stream *ds)
{
@ -1291,6 +1296,19 @@ static void perform_backward_seek(struct demux_internal *in)
pthread_mutex_lock(&in->lock);
}
// For incremental backward demuxing search work.
static void check_backward_seek(struct demux_internal *in)
{
in->back_any_need_recheck = false;
for (int n = 0; n < in->num_streams; n++) {
struct demux_stream *ds = in->streams[n]->ds;
if (ds->back_need_recheck)
find_backward_restart_pos(ds);
}
}
// Search for a packet to resume demuxing from.
// The implementation of this function is quite awkward, because the packet
// queue is a singly linked list without back links, while it needs to search
@ -1300,7 +1318,9 @@ static void find_backward_restart_pos(struct demux_stream *ds)
{
struct demux_internal *in = ds->in;
assert(ds->back_restarting);
ds->back_need_recheck = false;
if (!ds->back_restarting)
return;
struct demux_packet *first = ds->reader_head;
struct demux_packet *last = ds->queue->tail;
@ -1497,8 +1517,20 @@ resume_earlier:
}
if (ds->back_seek_pos != MP_NOPTS_VALUE) {
ds->back_seek_pos -= in->opts->back_seek_size;
in->need_back_seek = true;
struct demux_packet *t =
find_seek_target(ds->queue, ds->back_seek_pos - 0.001, 0);
if (t && t != ds->reader_head) {
double pts;
compute_keyframe_times(t, &pts, NULL);
ds->back_seek_pos = MP_PTS_MIN(ds->back_seek_pos, pts);
ds_clear_reader_state(ds, false);
ds->reader_head = t;
ds->back_need_recheck = true;
in->back_any_need_recheck = true;
} else {
ds->back_seek_pos -= in->opts->back_seek_size;
in->need_back_seek = true;
}
}
}
@ -2309,6 +2341,10 @@ static bool thread_work(struct demux_internal *in)
perform_backward_seek(in);
return true;
}
if (in->back_any_need_recheck) {
check_backward_seek(in);
return true;
}
if (in->seeking) {
execute_seek(in);
return true;