From f7678575a5d7f598abf267cb303e0a74db276f27 Mon Sep 17 00:00:00 2001 From: wm4 Date: Wed, 10 Jul 2019 21:34:01 +0200 Subject: [PATCH] recorder: always mux all packets on discont/close This is the muxer used by all 3 stream recording features (why are there so many?). It tried hard to avoid writing broken files. In particular, it buffered packets until it new there was a keyframe packet (which, in mpv's/FFmpeg's definition, mean seek points from which decoding can resume), or final EOF. The danger that was probably considered here was that due to video frame reordering, not muxing some trailing, missing packets of a keyframe range could lead to broken decoding or skipped frames, so better discard packets belonging to an incomplete range. Sounds like a good idea so far. Unfortunately, this will drop an entire keyframe range even if the current packet run is complete and mp_recorder_mark_discontinuity() is called, simply because recorder.c can not know that the next packet would have been a keyframe. It seems better to mux all packets to avoid losing valid data, even if it means that sometimes packets/frames will be missing from the file. It benefits especially the dump-cache command, which will call the function to signal a discontinuity after every range. Before this commit, it discarded the last packets, even if they were perfectly fine. (An alternative solution for dump-cache would have been a second discontinuity marker function, that communicates that the current packet range is complete. But this commit's solution is simpler and overall more robust, at the danger of producing more semi-broken files.) This may make some of the complex buffering/waiting logic in recorder.c pointless. Untested (in this final form). --- common/recorder.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/common/recorder.c b/common/recorder.c index 213caab6cb..ad9c34e24c 100644 --- a/common/recorder.c +++ b/common/recorder.c @@ -293,8 +293,6 @@ void mp_recorder_destroy(struct mp_recorder *priv) if (priv->opened) { for (int n = 0; n < priv->num_streams; n++) { struct mp_recorder_sink *rst = priv->streams[n]; - if (!rst->proper_eof) - continue; mux_packets(rst, true); } @@ -320,6 +318,7 @@ void mp_recorder_mark_discontinuity(struct mp_recorder *priv) for (int n = 0; n < priv->num_streams; n++) { struct mp_recorder_sink *rst = priv->streams[n]; + mux_packets(rst, true); rst->discont = true; rst->proper_eof = false; }