Skip to content

Commit

Permalink
vp8: always update next_framep[] before returning from decode_frame().
Browse files Browse the repository at this point in the history
Also slightly move around code not allocate a new frame if we won't
decode it. This prevents us from putting undecoded frames in frame
pointers, which (in mt decoding) other threads will use and wait on
as references, causing a deadlock (if we skipped decoding) or a crash
(if we didn't initialized next_framep[] at all).

Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
  • Loading branch information
rbultje committed Feb 7, 2012
1 parent 722410a commit fb90785
Showing 1 changed file with 19 additions and 14 deletions.
33 changes: 19 additions & 14 deletions libavcodec/vp8.c
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,7 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
release_queued_segmaps(s, 0);

if ((ret = decode_frame_header(s, avpkt->data, avpkt->size)) < 0)
return ret;
goto err;

prev_frame = s->framep[VP56_FRAME_CURRENT];

Expand All @@ -1583,6 +1583,7 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,

if (avctx->skip_frame >= skip_thresh) {
s->invisible = 1;
memcpy(&s->next_framep[0], &s->framep[0], sizeof(s->framep[0]) * 4);
goto skip_decode;
}
s->deblock_filter = s->filter.level && avctx->skip_loop_filter < skip_thresh;
Expand Down Expand Up @@ -1612,12 +1613,23 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
if (curframe->data[0])
vp8_release_frame(s, curframe, 1, 0);

// Given that arithmetic probabilities are updated every frame, it's quite likely
// that the values we have on a random interframe are complete junk if we didn't
// start decode on a keyframe. So just don't display anything rather than junk.
if (!s->keyframe && (!s->framep[VP56_FRAME_PREVIOUS] ||
!s->framep[VP56_FRAME_GOLDEN] ||
!s->framep[VP56_FRAME_GOLDEN2])) {
av_log(avctx, AV_LOG_WARNING, "Discarding interframe without a prior keyframe!\n");
ret = AVERROR_INVALIDDATA;
goto err;
}

curframe->key_frame = s->keyframe;
curframe->pict_type = s->keyframe ? AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
curframe->reference = referenced ? 3 : 0;
if ((ret = vp8_alloc_frame(s, curframe))) {
av_log(avctx, AV_LOG_ERROR, "get_buffer() failed!\n");
return ret;
goto err;
}

// check if golden and altref are swapped
Expand All @@ -1640,16 +1652,6 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,

ff_thread_finish_setup(avctx);

// Given that arithmetic probabilities are updated every frame, it's quite likely
// that the values we have on a random interframe are complete junk if we didn't
// start decode on a keyframe. So just don't display anything rather than junk.
if (!s->keyframe && (!s->framep[VP56_FRAME_PREVIOUS] ||
!s->framep[VP56_FRAME_GOLDEN] ||
!s->framep[VP56_FRAME_GOLDEN2])) {
av_log(avctx, AV_LOG_WARNING, "Discarding interframe without a prior keyframe!\n");
return AVERROR_INVALIDDATA;
}

s->linesize = curframe->linesize[0];
s->uvlinesize = curframe->linesize[1];

Expand Down Expand Up @@ -1759,20 +1761,23 @@ static int vp8_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
}

ff_thread_report_progress(curframe, INT_MAX, 0);
memcpy(&s->framep[0], &s->next_framep[0], sizeof(s->framep[0]) * 4);

skip_decode:
// if future frames don't use the updated probabilities,
// reset them to the values we saved
if (!s->update_probabilities)
s->prob[0] = s->prob[1];

memcpy(&s->framep[0], &s->next_framep[0], sizeof(s->framep[0]) * 4);

if (!s->invisible) {
*(AVFrame*)data = *curframe;
*data_size = sizeof(AVFrame);
}

return avpkt->size;
err:
memcpy(&s->next_framep[0], &s->framep[0], sizeof(s->framep[0]) * 4);
return ret;
}

static av_cold int vp8_decode_init(AVCodecContext *avctx)
Expand Down

0 comments on commit fb90785

Please sign in to comment.