demux: don't access stream while lock is held

Although this is fine when the stream cache is active (which caches
these and returns the result immediately), it seems cleaner not to
rely on this detail.

Remove the update_cache() call from demux_thread(), because it's sort
of in the way. I forgot why it exists, and there's probably no good
reason for it to exist anyway.
This commit is contained in:
wm4 2014-10-24 15:56:45 +02:00
parent 07cca2500e
commit 480f82fa96
1 changed files with 30 additions and 22 deletions

View File

@ -387,9 +387,8 @@ static bool read_packet(struct demux_internal *in)
pthread_mutex_unlock(&in->lock); pthread_mutex_unlock(&in->lock);
struct demuxer *demux = in->d_thread; struct demuxer *demux = in->d_thread;
bool eof = !demux->desc->fill_buffer || demux->desc->fill_buffer(demux) <= 0; bool eof = !demux->desc->fill_buffer || demux->desc->fill_buffer(demux) <= 0;
pthread_mutex_lock(&in->lock);
update_cache(in); update_cache(in);
pthread_mutex_lock(&in->lock);
if (eof) { if (eof) {
for (int n = 0; n < in->d_buffer->num_streams; n++) { for (int n = 0; n < in->d_buffer->num_streams; n++) {
@ -480,7 +479,6 @@ static void *demux_thread(void *pctx)
if (read_packet(in)) if (read_packet(in))
continue; // read_packet unlocked, so recheck conditions continue; // read_packet unlocked, so recheck conditions
} }
update_cache(in);
pthread_cond_signal(&in->wakeup); pthread_cond_signal(&in->wakeup);
pthread_cond_wait(&in->wakeup, &in->lock); pthread_cond_wait(&in->wakeup, &in->lock);
} }
@ -758,10 +756,10 @@ void demux_changed(demuxer_t *demuxer, int events)
demuxer->events |= events; demuxer->events |= events;
pthread_mutex_lock(&in->lock);
update_cache(in); update_cache(in);
pthread_mutex_lock(&in->lock);
if (demuxer->events & DEMUX_EVENT_INIT) if (demuxer->events & DEMUX_EVENT_INIT)
demuxer_sort_chapters(demuxer); demuxer_sort_chapters(demuxer);
if (demuxer->events & (DEMUX_EVENT_METADATA | DEMUX_EVENT_STREAMS)) if (demuxer->events & (DEMUX_EVENT_METADATA | DEMUX_EVENT_STREAMS))
@ -779,9 +777,10 @@ void demux_update(demuxer_t *demuxer)
assert(demuxer == demuxer->in->d_user); assert(demuxer == demuxer->in->d_user);
struct demux_internal *in = demuxer->in; struct demux_internal *in = demuxer->in;
pthread_mutex_lock(&in->lock);
if (!in->threading) if (!in->threading)
update_cache(in); update_cache(in);
pthread_mutex_lock(&in->lock);
demux_copy(demuxer, in->d_buffer); demux_copy(demuxer, in->d_buffer);
if (in->stream_metadata && (demuxer->events & DEMUX_EVENT_METADATA)) if (in->stream_metadata && (demuxer->events & DEMUX_EVENT_METADATA))
mp_tags_merge(demuxer->metadata, in->stream_metadata); mp_tags_merge(demuxer->metadata, in->stream_metadata);
@ -1101,34 +1100,43 @@ double demuxer_get_time_length(struct demuxer *demuxer)
return -1; return -1;
} }
// must be called locked // must be called not locked
static void update_cache(struct demux_internal *in) static void update_cache(struct demux_internal *in)
{ {
struct demuxer *demuxer = in->d_thread; struct demuxer *demuxer = in->d_thread;
struct stream *stream = demuxer->stream; struct stream *stream = demuxer->stream;
in->time_length = -1; // Don't lock while querying the stream.
double time_length = -1;
struct mp_tags *stream_metadata = NULL;
int64_t stream_size = -1;
int64_t stream_cache_size = -1;
int64_t stream_cache_fill = -1;
int stream_cache_idle = -1;
if (demuxer->desc->control) { if (demuxer->desc->control) {
demuxer->desc->control(demuxer, DEMUXER_CTRL_GET_TIME_LENGTH, demuxer->desc->control(demuxer, DEMUXER_CTRL_GET_TIME_LENGTH,
&in->time_length); &time_length);
} }
struct mp_tags *s_meta = NULL; stream_control(stream, STREAM_CTRL_GET_METADATA, &stream_metadata);
stream_control(stream, STREAM_CTRL_GET_METADATA, &s_meta); stream_control(stream, STREAM_CTRL_GET_SIZE, &stream_size);
if (s_meta) { stream_control(stream, STREAM_CTRL_GET_CACHE_SIZE, &stream_cache_size);
stream_control(stream, STREAM_CTRL_GET_CACHE_FILL, &stream_cache_fill);
stream_control(stream, STREAM_CTRL_GET_CACHE_IDLE, &stream_cache_idle);
pthread_mutex_lock(&in->lock);
in->time_length = time_length;
in->stream_size = stream_size;
in->stream_cache_size = stream_cache_size;
in->stream_cache_fill = stream_cache_fill;
in->stream_cache_idle = stream_cache_idle;
if (stream_metadata) {
talloc_free(in->stream_metadata); talloc_free(in->stream_metadata);
in->stream_metadata = talloc_steal(in, s_meta); in->stream_metadata = talloc_steal(in, stream_metadata);
in->d_buffer->events |= DEMUX_EVENT_METADATA; in->d_buffer->events |= DEMUX_EVENT_METADATA;
} }
pthread_mutex_unlock(&in->lock);
in->stream_size = -1;
stream_control(stream, STREAM_CTRL_GET_SIZE, &in->stream_size);
in->stream_cache_size = -1;
stream_control(stream, STREAM_CTRL_GET_CACHE_SIZE, &in->stream_cache_size);
in->stream_cache_fill = -1;
stream_control(stream, STREAM_CTRL_GET_CACHE_FILL, &in->stream_cache_fill);
in->stream_cache_idle = -1;
stream_control(stream, STREAM_CTRL_GET_CACHE_IDLE, &in->stream_cache_idle);
} }
// must be called locked // must be called locked