From 05b2cd08dcaddb9ea6224f3ad3a32e1d967d0a98 Mon Sep 17 00:00:00 2001 From: wm4 Date: Sun, 6 Mar 2016 14:50:36 +0100 Subject: [PATCH] sub: make preloading more robust Subtitles can be preloaded, which means they're fully read and copied into ASS_Track. This in turn is mainly for the sake of being able to do subtitle seeking (when it comes down to it, subtitle seeking is the cause for most trouble here). Commit a714f8e92 broke preloaded subtitles which have events with unknown duration, such as some MicroDVD samples. The event list gets cleared on every seek, so the property of being preloaded obviously gets lost. Fix this by moving most of the preloading logic to dec_sub.c. If the subtitle list gets cleared, they are not considered preloaded anymore, and the logic for demuxed subtitles is used. As another minor thing, preloadeding subtitles did neither disable the demux stream, nor did it discard packets. Thus you could get queue overflows in theory (harmless, but annoying). Fix this by explicitly discarding packets in preloaded mode. In summary, now the only difference between preloaded and normal demuxing are: 1. a seek is issued, and all packets are read on start 2. during playback, discard the packets instead of feeding them to the subtitle decoder This is still petty annoying. It would be nice if maintaining the subtitle index (and maybe a subtitle packet cache for instant subtitle presentation when seeking back) could be maintained in the demuxer instead. Half of all file formats with interleaved subtitles have this anyway (mp4, mkv muxed with newer mkvmerge). --- player/core.h | 4 ---- player/sub.c | 11 ++++------- sub/dec_sub.c | 25 ++++++++++++++++--------- sub/dec_sub.h | 3 ++- sub/sd.h | 4 ++++ sub/sd_ass.c | 2 ++ 6 files changed, 28 insertions(+), 21 deletions(-) diff --git a/player/core.h b/player/core.h index f26cacbb51..21129d6ea3 100644 --- a/player/core.h +++ b/player/core.h @@ -147,10 +147,6 @@ struct track { struct vo_chain *vo_c; struct ao_chain *ao_c; struct lavfi_pad *sink; - - // For external subtitles, which are read fully on init. Do not attempt - // to read packets from them. - bool preloaded; }; // Summarizes video filtering and output. diff --git a/player/sub.c b/player/sub.c index 426e31bc7a..69c1dbbd19 100644 --- a/player/sub.c +++ b/player/sub.c @@ -99,18 +99,15 @@ static bool update_subtitle(struct MPContext *mpctx, double video_pts, video_pts -= opts->sub_delay; - if (!track->preloaded && track->demuxer->fully_read && !opts->sub_clear_on_seek) - { + if (track->demuxer->fully_read && sub_can_preload(dec_sub)) { // Assume fully_read implies no interleaved audio/video streams. // (Reading packets will change the demuxer position.) demux_seek(track->demuxer, 0, 0); - track->preloaded = sub_read_all_packets(track->d_sub); + sub_preload(dec_sub); } - if (!track->preloaded) { - if (!sub_read_packets(dec_sub, video_pts)) - return false; - } + if (!sub_read_packets(dec_sub, video_pts)) + return false; // Handle displaying subtitles on terminal; never done for secondary subs if (mpctx->current_track[0][STREAM_SUB] == track && !mpctx->video_out) diff --git a/sub/dec_sub.c b/sub/dec_sub.c index fbce829f5f..be55fc0906 100644 --- a/sub/dec_sub.c +++ b/sub/dec_sub.c @@ -53,6 +53,7 @@ struct dec_sub { struct sh_stream *sh; double last_pkt_pts; + bool preload_attempted; struct mp_codec_params *codec; double start, end; @@ -96,6 +97,7 @@ static struct sd *init_decoder(struct dec_sub *sub) .driver = driver, .attachments = sub->attachments, .codec = sub->codec, + .preload_ok = true, }; if (sd->driver->init(sd) >= 0) @@ -167,16 +169,20 @@ static void update_segment(struct dec_sub *sub) } } -// Read all packets from the demuxer and decode/add them. Returns false if -// there are circumstances which makes this not possible. -bool sub_read_all_packets(struct dec_sub *sub) +bool sub_can_preload(struct dec_sub *sub) +{ + bool r; + pthread_mutex_lock(&sub->lock); + r = sub->sd->driver->accept_packets_in_advance && !sub->preload_attempted; + pthread_mutex_unlock(&sub->lock); + return r; +} + +void sub_preload(struct dec_sub *sub) { pthread_mutex_lock(&sub->lock); - if (!sub->sd->driver->accept_packets_in_advance) { - pthread_mutex_unlock(&sub->lock); - return false; - } + sub->preload_attempted = true; for (;;) { struct demux_packet *pkt = demux_read_packet(sub->sh); @@ -187,7 +193,6 @@ bool sub_read_all_packets(struct dec_sub *sub) } pthread_mutex_unlock(&sub->lock); - return true; } // Read packets from the demuxer stream passed to sub_create(). Return true if @@ -230,7 +235,9 @@ bool sub_read_packets(struct dec_sub *sub, double video_pts) break; } - sub->sd->driver->decode(sub->sd, pkt); + if (!(sub->preload_attempted && sub->sd->preload_ok)) + sub->sd->driver->decode(sub->sd, pkt); + talloc_free(pkt); } pthread_mutex_unlock(&sub->lock); diff --git a/sub/dec_sub.h b/sub/dec_sub.h index cedb140e79..63603e2174 100644 --- a/sub/dec_sub.h +++ b/sub/dec_sub.h @@ -32,7 +32,8 @@ void sub_destroy(struct dec_sub *sub); void sub_lock(struct dec_sub *sub); void sub_unlock(struct dec_sub *sub); -bool sub_read_all_packets(struct dec_sub *sub); +bool sub_can_preload(struct dec_sub *sub); +void sub_preload(struct dec_sub *sub); bool sub_read_packets(struct dec_sub *sub, double video_pts); void sub_get_bitmaps(struct dec_sub *sub, struct mp_osd_res dim, double pts, struct sub_bitmaps *res); diff --git a/sub/sd.h b/sub/sd.h index 92bbf906fd..fe64163b1b 100644 --- a/sub/sd.h +++ b/sub/sd.h @@ -19,6 +19,10 @@ struct sd { struct attachment_list *attachments; struct mp_codec_params *codec; + + // Set to false as soon as the decoder discards old subtitle events. + // (only needed if sd_functions.accept_packets_in_advance == false) + bool preload_ok; }; struct sd_functions { diff --git a/sub/sd_ass.c b/sub/sd_ass.c index f8c9abebb2..23f0c882f8 100644 --- a/sub/sd_ass.c +++ b/sub/sd_ass.c @@ -452,6 +452,7 @@ static void get_bitmaps(struct sd *sd, struct mp_osd_res dim, double pts, if (ctx->duration_unknown && pts != MP_NOPTS_VALUE) { mp_ass_flush_old_events(track, ts); ctx->num_seen_packets = 0; + sd->preload_ok = false; } if (no_ass) fill_plaintext(sd, pts); @@ -612,6 +613,7 @@ static void reset(struct sd *sd) if (sd->opts->sub_clear_on_seek || ctx->duration_unknown) { ass_flush_events(ctx->ass_track); ctx->num_seen_packets = 0; + sd->preload_ok = false; } if (ctx->converter) lavc_conv_reset(ctx->converter);