Skip to content

Commit

Permalink
demux: close underlying stream if it's fully read anyway
Browse files Browse the repository at this point in the history
This is for text subtitles. libavformat currently always reads text
subtitles completely on init. This means the underlying stream is
useless and will consume resources for various reasons (network
connection, file handles, cache memory).

Take care of this by closing the underlying stream if we think the
demuxer has read everything. Since libavformat doesn't export whether it
did (or whether it may access the stream again in the future), we rely
on a whitelist. Also, instead of setting the stream to NULL or so, set
it to an empty dummy stream. This way we don't have to litter the code
with NULL checks.

demux_lavf.c needs extra changes, because it tries to do clever things
for the sake of subtitle charset conversion.

The main reason we keep the demuxer etc. open is because we fell for
libavformat being so generic, and we tried to remove corresponding
special-cases in the higher-level player code. Some of this is forced
due to ass/srt mkv/mp4 demuxing being very similar to external text
files. In the future it might be better to do this in a more
straight-forward way, such as reading text subtitles into libass and
then discarding the demuxer entirely, but for aforementioned reasons
this could be more of a mess than the solution introduced by this
commit.

Probably fixes mpv-player#3456.
  • Loading branch information
wm4 committed Aug 26, 2016
1 parent 4121016 commit bc97d60
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 3 deletions.
22 changes: 21 additions & 1 deletion demux/demux.c
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,23 @@ static void demux_init_cuesheet(struct demuxer *demuxer)
}
}

static void demux_maybe_replace_stream(struct demuxer *demuxer)
{
struct demux_internal *in = demuxer->in;
assert(!in->threading && demuxer == in->d_user);

if (demuxer->fully_read) {
MP_VERBOSE(demuxer, "assuming demuxer read all data; closing stream\n");
free_stream(demuxer->stream);
demuxer->stream = open_memory_stream(NULL, 0); // dummy
in->d_thread->stream = demuxer->stream;
in->d_buffer->stream = demuxer->stream;

if (demuxer->desc->control)
demuxer->desc->control(in->d_thread, DEMUXER_CTRL_REPLACE_STREAM, NULL);
}
}

static struct demuxer *open_given_type(struct mpv_global *global,
struct mp_log *log,
const struct demuxer_desc *desc,
Expand Down Expand Up @@ -1313,6 +1330,7 @@ struct demuxer *demux_open(struct stream *stream, struct demuxer_params *params,
// Convenience function: open the stream, enable the cache (according to params
// and global opts.), open the demuxer.
// (use free_demuxer_and_stream() to free the underlying stream too)
// Also for some reason may close the opened stream if it's not needed.
struct demuxer *demux_open_url(const char *url,
struct demuxer_params *params,
struct mp_cancel *cancel,
Expand All @@ -1331,7 +1349,9 @@ struct demuxer *demux_open_url(const char *url,
if (!params->disable_cache)
stream_enable_cache(&s, &opts->stream_cache);
struct demuxer *d = demux_open(s, params, global);
if (!d) {
if (d) {
demux_maybe_replace_stream(d);
} else {
params->demuxer_failed = true;
free_stream(s);
}
Expand Down
2 changes: 2 additions & 0 deletions demux/demux.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ enum demux_ctrl {
DEMUXER_CTRL_STREAM_CTRL,
DEMUXER_CTRL_GET_READER_STATE,
DEMUXER_CTRL_GET_BITRATE_STATS, // double[STREAM_TYPE_COUNT]
DEMUXER_CTRL_REPLACE_STREAM,
};

struct demux_ctrl_reader_state {
Expand Down Expand Up @@ -220,6 +221,7 @@ typedef struct demuxer {
// thread-safe, only the demuxer is allowed to access the stream directly.
// You can freely use demux_stream_control() to send STREAM_CTRLs, or use
// demux_pause() to get exclusive access to the stream.
// Also note that the stream can get replaced if fully_read is set.
struct stream *stream;
} demuxer_t;

Expand Down
13 changes: 11 additions & 2 deletions demux/demux_lavf.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ static const struct format_hack format_hacks[] = {

typedef struct lavf_priv {
struct stream *stream;
bool own_stream;
char *filename;
struct format_hack format_hack;
AVInputFormat *avif;
Expand Down Expand Up @@ -290,8 +291,10 @@ static void convert_charset(struct demuxer *demuxer)
if (conv.start)
data = conv;
}
if (data.start)
if (data.start) {
priv->stream = open_memory_stream(data.start, data.len);
priv->own_stream = true;
}
talloc_free(alloc);
}

Expand Down Expand Up @@ -1068,6 +1071,12 @@ static int demux_lavf_control(demuxer_t *demuxer, int cmd, void *arg)
av_seek_frame(priv->avfc, 0, stream_tell(priv->stream),
AVSEEK_FLAG_BYTE);
return DEMUXER_CTRL_OK;
case DEMUXER_CTRL_REPLACE_STREAM:
if (priv->own_stream)
free_stream(priv->stream);
priv->own_stream = false;
priv->stream = demuxer->stream;
return DEMUXER_CTRL_OK;
default:
return DEMUXER_CTRL_NOTIMPL;
}
Expand All @@ -1092,7 +1101,7 @@ static void demux_close_lavf(demuxer_t *demuxer)
#endif
}
}
if (priv->stream != demuxer->stream)
if (priv->own_stream)
free_stream(priv->stream);
talloc_free(priv);
demuxer->priv = NULL;
Expand Down

0 comments on commit bc97d60

Please sign in to comment.