Skip to content

Commit

Permalink
Fix a possible crash in avformat probe (#944)
Browse files Browse the repository at this point in the history 
* Fix a possible crash in avformat probe

Probe was accessing the cached avformat context from a store pointer
(self->child) rather than through the cache. This can result in
accesssing released memory if the cache deleted the context.

The context pointer should not be saved at all and should only be
accessed through the cache system.

This change includes a rewrite of the video index change detection
so that probe() will not have to access the avformat context at all.

This also adds an optimization that if metadata is loaded from XML,
and nothing else has changed, then the probe can be skipped.

* Use meta.media.nb_streams instead of meta.media.stream.0.codec

Stream 0 may not exist. And nb_streams should always be present
  • Loading branch information
@bmatherly
bmatherly committed Sep 4, 2023
1 parent 54d3499 commit 2266e5c
Showing 1 changed file with 27 additions and 27 deletions.
54 changes: 27 additions & 27 deletions src/modules/avformat/producer_avformat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ struct producer_avformat_s
int video_index;
int64_t first_pts;
atomic_int_fast64_t last_position;
int probe_complete;
int video_seekable;
int seekable; /// This one is used for both audio and file level seekability.
atomic_int_fast64_t current_position;
Expand Down Expand Up @@ -1505,28 +1504,28 @@ static int pick_av_pixel_format(int *pix_fmt, int full_range)

static void property_changed(mlt_service owner, producer_avformat self, char *name)
{
if (self && name) {
if (self && name && self->parent) {
mlt_properties properties = MLT_PRODUCER_PROPERTIES(self->parent);
if (!strcmp("color_range", name)) {
if (self->video_codec
&& !av_opt_set(self->video_codec,
name,
mlt_properties_get(MLT_PRODUCER_PROPERTIES(self->parent), name),
mlt_properties_get(properties, name),
AV_OPT_SEARCH_CHILDREN)) {
if (self->full_range != (self->video_codec->color_range == AVCOL_RANGE_JPEG)) {
self->full_range = self->video_codec->color_range == AVCOL_RANGE_JPEG;
self->reset_image_cache = 1;
}
}
} else if (!strcmp("force_full_range", name) || !strcmp("set.force_full_luma", name)) {
mlt_properties properties = MLT_PRODUCER_PROPERTIES(self->parent);
if (self->full_range != mlt_properties_get_int(properties, name)) {
self->full_range = mlt_properties_get_int(properties, name);
self->reset_image_cache = 1;
}
} else if (!strcmp("force_progressive", name) || !strcmp("force_tff", name)) {
self->reset_image_cache = 1;
} else if (!strcmp("autorotate", name)) {
self->autorotate = mlt_properties_get_int(MLT_PRODUCER_PROPERTIES(self->parent), name);
self->autorotate = mlt_properties_get_int(properties, name);
if (self->video_index != -1) {
mlt_service_lock(MLT_PRODUCER_SERVICE(self->parent));
avfilter_graph_free(&self->vfilter_graph);
Expand All @@ -1536,6 +1535,10 @@ static void property_changed(mlt_service owner, producer_avformat self, char *na
self->reset_image_cache = 1;
mlt_service_unlock(MLT_PRODUCER_SERVICE(self->parent));
}
} else if (!strcmp("video_index", name) || !strcmp("vstream", name)) {
if (mlt_properties_get_int(properties, "_probe_complete")) {
mlt_properties_set_int(properties, "_probe_complete", 0);
}
}
}
}
Expand Down Expand Up @@ -2540,7 +2543,7 @@ static int producer_get_image(mlt_frame frame,
// Set immutable properties of the selected track's (or overridden) source attributes.
mlt_properties_set_int(properties, "meta.media.top_field_first", self->top_field_first);
mlt_properties_set_int(properties, "meta.media.progressive", self->progressive);
self->probe_complete = 1;
mlt_properties_set_int(properties, "_probe_complete", 1);
mlt_service_unlock(MLT_PRODUCER_SERVICE(producer));

mlt_log_timings_end(NULL, __FUNCTION__);
Expand Down Expand Up @@ -2922,7 +2925,7 @@ static void producer_set_up_video(producer_avformat self, mlt_frame frame)
if (context && index > -1 && index != self->video_index) {
// Reset the video properties if the index changed
self->video_index = index;
self->probe_complete = 0;
mlt_properties_set_int(properties, "_probe_complete", 0);
pthread_mutex_lock(&self->open_mutex);
avcodec_free_context(&self->video_codec);
set_up_discard(self, self->audio_index, index);
Expand Down Expand Up @@ -3682,7 +3685,7 @@ static void producer_set_up_audio(producer_avformat self, mlt_frame frame)
1,
0);
context = self->audio_format;
self->probe_complete = 0;
mlt_properties_set_int(properties, "_probe_complete", 0);
}
index = pick_audio_stream(self);

Expand Down Expand Up @@ -3741,7 +3744,6 @@ static int producer_get_frame(mlt_producer producer, mlt_frame_ptr frame, int in
// If cache miss
if (!self) {
self = calloc(1, sizeof(struct producer_avformat_s));
producer->child = self;
self->parent = producer;
mlt_service_cache_put(service,
"producer_avformat",
Expand Down Expand Up @@ -3784,7 +3786,8 @@ static int producer_get_frame(mlt_producer producer, mlt_frame_ptr frame, int in
mlt_position position = mlt_producer_frame(producer);
mlt_properties_set_position(frame_properties, "original_position", position);

if (!self->probe_complete && self->video_index < 0) {
if (!mlt_properties_get_int(MLT_PRODUCER_PROPERTIES(producer), "_probe_complete")
&& self->video_index < 0) {
// If video index is valid, get_image() must be called before the probe is complete
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "meta.media.width");
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "meta.media.height");
Expand All @@ -3794,7 +3797,6 @@ static int producer_get_frame(mlt_producer producer, mlt_frame_ptr frame, int in
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "meta.media.sample_aspect_num");
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "meta.media.sample_aspect_den");
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "meta.media.top_field_first");
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "meta.media.progressive");
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "meta.media.variable_frame_rate");
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "meta.media.colorspace");
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "meta.media.color_trc");
Expand All @@ -3803,7 +3805,7 @@ static int producer_get_frame(mlt_producer producer, mlt_frame_ptr frame, int in
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "width");
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "height");
mlt_properties_clear(MLT_PRODUCER_PROPERTIES(producer), "format");
self->probe_complete = 1;
mlt_properties_set_int(MLT_PRODUCER_PROPERTIES(producer), "_probe_complete", 1);
}

// Calculate the next timecode
Expand All @@ -3814,33 +3816,31 @@ static int producer_get_frame(mlt_producer producer, mlt_frame_ptr frame, int in

static int producer_probe(mlt_producer producer)
{
producer_avformat self = producer->child;
int error = 0;
mlt_properties properties = MLT_PRODUCER_PROPERTIES(producer);

// Update the video properties if the index changed
int video_index = pick_video_stream(self);

if (self->video_format && video_index > -1 && video_index != self->video_index)
self->probe_complete = 0;

// Update the audio properties if the index changed
int audio_index = pick_audio_stream(self);

if (self->audio_format && audio_index > -1 && audio_index != self->audio_index)
self->probe_complete = 0;

if (self->probe_complete) {
if (mlt_properties_get_int(properties, "_probe_complete")) {
return error;
}

if (!mlt_properties_exists(properties, "_probe_complete")) {
// If metadata was loaded from XML, no need to probe
int video_in_use = mlt_properties_get_int(properties, "vstream") > -1;
if (video_in_use && mlt_properties_exists(properties, "meta.media.progressive")) {
return error;
} else if (!video_in_use && mlt_properties_exists(properties, "meta.media.nb_streams")) {
return error;
}
}

mlt_frame fr = NULL;
mlt_position save_position = mlt_producer_position(producer);

// Call producer_get_frame() directly so that the underlying service will not attach any normalizers
mlt_service_lock(MLT_PRODUCER_SERVICE(producer));
error = producer_get_frame(producer, &fr, 0);
mlt_service_unlock(MLT_PRODUCER_SERVICE(producer));
if (!error && fr && self->video_index > -1) {
if (!error && fr && mlt_properties_get_int(properties, "vstream") > -1) {
// Some video metadata is not exposed until after the first get_image call.
uint8_t *buffer = NULL;
mlt_image_format fmt = mlt_image_none;
Expand Down

0 comments on commit 2266e5c

Please sign in to comment.