From 4e25feda0d5e084761a9935de7c1e592e86de94f Mon Sep 17 00:00:00 2001 From: wm4 Date: Tue, 10 Jan 2017 14:03:41 +0100 Subject: player: change aspects of cover art handling Cover art handling is a disgusting hack that causes a mess in all components. And this will stay this way. This is the Xth time I've changed cover art handling, and that will probably also continue. But change the code such that cover art is injected into the demux packet stream, instead of having an explicit special case it in the decoder glue code. (This is somewhat more similar to the cover art hack in libavformat.) To avoid that the over art picture is decoded again on each seek, we need some additional "caching" in player/video.c. Decoding it after each seek would work as well, but since cover art pictures can be pretty huge, it's probably ok to invest some lines of code into caching it. One weird thing is that the cover art packet will remain queued after seeks, but that is probably not an issue. In exchange, we can drop the dec_video.c code, which is pretty convenient for one of the following commits. This code duplicates a bunch of lower-level decode calls and does icky messing with this weird state stuff, so I'm glad it goes away. --- demux/demux.c | 18 +++++++++++++++++- player/core.h | 2 ++ player/video.c | 26 +++++++++++++++++++++----- video/decode/dec_video.c | 17 ----------------- video/decode/dec_video.h | 1 - 5 files changed, 40 insertions(+), 24 deletions(-) diff --git a/demux/demux.c b/demux/demux.c index a94d98e10f..2195246ac8 100644 --- a/demux/demux.c +++ b/demux/demux.c @@ -205,6 +205,9 @@ struct demux_stream { struct demux_packet *head; struct demux_packet *tail; + struct demux_packet *attached_picture; + bool attached_picture_added; + // for closed captions (demuxer_feed_caption) struct sh_stream *cc; }; @@ -243,6 +246,7 @@ static void ds_flush(struct demux_stream *ds) ds->last_pos = -1; ds->last_dts = MP_NOPTS_VALUE; ds->correct_dts = ds->correct_pos = true; + ds->attached_picture_added = false; } void demux_set_ts_offset(struct demuxer *demuxer, double offset) @@ -291,6 +295,8 @@ void demux_add_sh_stream(struct demuxer *demuxer, struct sh_stream *sh) if (!sh->codec->codec) sh->codec->codec = ""; + sh->ds->attached_picture = sh->attached_picture; + sh->index = in->num_streams; if (sh->ff_index < 0) sh->ff_index = sh->index; @@ -755,6 +761,13 @@ static void *demux_thread(void *pctx) static struct demux_packet *dequeue_packet(struct demux_stream *ds) { + if (ds->attached_picture) { + ds->eof = true; + if (ds->attached_picture_added) + return NULL; + ds->attached_picture_added = true; + return demux_copy_packet(ds->attached_picture); + } if (!ds->head) return NULL; struct demux_packet *pkt = ds->head; @@ -800,18 +813,21 @@ static struct demux_packet *dequeue_packet(struct demux_stream *ds) // Whether to avoid actively demuxing new packets to find a new packet on the // given stream. +// Attached pictures (cover art) should never actively read. // Sparse packets (Subtitles) interleaved with other non-sparse packets (video, // audio) should never be read actively, meaning the demuxer thread does not // try to exceed default readahead in order to find a new packet. static bool use_lazy_packet_reading(struct demux_stream *ds) { + if (ds->attached_picture) + return true; if (ds->type != STREAM_SUB) return false; // Subtitles are only lazily read if there's at least 1 other actively read // stream. for (int n = 0; n < ds->in->num_streams; n++) { struct demux_stream *s = ds->in->streams[n]->ds; - if (s->type != STREAM_SUB && s->selected && !s->eof) + if (s->type != STREAM_SUB && s->selected && !s->eof && !s->attached_picture) return true; } return false; diff --git a/player/core.h b/player/core.h index d8640d9ef0..d7510a79fe 100644 --- a/player/core.h +++ b/player/core.h @@ -178,6 +178,8 @@ struct vo_chain { // - video consists of a single picture, which should be shown only once // - do not sync audio to video in any way bool is_coverart; + // Just to avoid decoding the coverart picture again after a seek. + struct mp_image *cached_coverart; }; // Like vo_chain, for audio. diff --git a/player/video.c b/player/video.c index 4621d19cb4..78d10fd107 100644 --- a/player/video.c +++ b/player/video.c @@ -328,6 +328,10 @@ static void vo_chain_reset_state(struct vo_chain *vo_c) if (vo_c->video_src) video_reset(vo_c->video_src); + + // Prepare for continued playback after a seek. + if (!vo_c->input_mpi && vo_c->cached_coverart) + vo_c->input_mpi = mp_image_new_ref(vo_c->cached_coverart); } void reset_video_state(struct MPContext *mpctx) @@ -381,6 +385,7 @@ static void vo_chain_uninit(struct vo_chain *vo_c) lavfi_set_connected(vo_c->filter_src, false); mp_image_unrefp(&vo_c->input_mpi); + mp_image_unrefp(&vo_c->cached_coverart); vf_destroy(vo_c->vf); talloc_free(vo_c); // this does not free the VO @@ -682,14 +687,25 @@ static int video_decode_and_filter(struct MPContext *mpctx) return r; if (!vo_c->input_mpi) { - // Decode a new image, or at least feed the decoder a packet. - r = decode_image(mpctx); - if (r == VD_WAIT) - return r; + if (vo_c->cached_coverart) { + // Don't ever decode it twice, not even after seek resets. + // (On seek resets, input_mpi is set to the cached image.) + r = VD_EOF; + } else { + // Decode a new image, or at least feed the decoder a packet. + r = decode_image(mpctx); + if (r == VD_WAIT) + return r; + } } - if (vo_c->input_mpi) + + if (vo_c->input_mpi) { vo_c->input_format = vo_c->input_mpi->params; + if (vo_c->is_coverart && !vo_c->cached_coverart) + vo_c->cached_coverart = mp_image_new_ref(vo_c->input_mpi); + } + bool eof = !vo_c->input_mpi && (r == VD_EOF || r < 0); r = video_filter(mpctx, eof); if (r == VD_RECONFIG) // retry feeding decoded image diff --git a/video/decode/dec_video.c b/video/decode/dec_video.c index d9dbf0f326..95ca49250b 100644 --- a/video/decode/dec_video.c +++ b/video/decode/dec_video.c @@ -89,7 +89,6 @@ void video_uninit(struct dec_video *d_video) if (!d_video) return; mp_image_unrefp(&d_video->current_mpi); - mp_image_unrefp(&d_video->cover_art_mpi); if (d_video->vd_driver) { MP_VERBOSE(d_video, "Uninit video.\n"); d_video->vd_driver->uninit(d_video); @@ -382,22 +381,6 @@ void video_work(struct dec_video *d_video) if (d_video->current_mpi) return; - if (d_video->header->attached_picture) { - if (d_video->current_state == DATA_AGAIN && !d_video->cover_art_mpi) { - struct demux_packet *packet = - demux_copy_packet(d_video->header->attached_picture); - d_video->cover_art_mpi = decode_packet(d_video, packet, 0); - // Might need flush. - if (!d_video->cover_art_mpi) - d_video->cover_art_mpi = decode_packet(d_video, NULL, 0); - talloc_free(packet); - } - if (d_video->current_state != DATA_EOF) - d_video->current_mpi = mp_image_new_ref(d_video->cover_art_mpi); - d_video->current_state = DATA_EOF; - return; - } - if (!d_video->packet && !d_video->new_segment && demux_read_packet_async(d_video->header, &d_video->packet) == 0) { diff --git a/video/decode/dec_video.h b/video/decode/dec_video.h index 1d2b3f087e..9155a76155 100644 --- a/video/decode/dec_video.h +++ b/video/decode/dec_video.h @@ -75,7 +75,6 @@ struct dec_video { struct demux_packet *new_segment; struct demux_packet *packet; bool framedrop_enabled; - struct mp_image *cover_art_mpi; struct mp_image *current_mpi; int current_state; }; -- cgit v1.2.3