From 65fcd202428550a1ed64421ce742b564e6fbcd5c Mon Sep 17 00:00:00 2001 From: Richard Stanway Date: Thu, 28 Apr 2016 23:53:57 +0200 Subject: [PATCH] libff: Improved handling of EOF in the decoder threads (Also modifies obs-ffmpeg to handle empty frames on EOF) Previously the demuxer could hit EOF before the decoder threads are finished, resulting in truncated output. In the worse case scenario the demuxer could read small files before ff_decoder_refresh even has a chance to start the clocks, resulting in no output at all. --- deps/libff/libff/ff-audio-decoder.c | 24 +++++++++++++++++++----- deps/libff/libff/ff-decoder.c | 5 ++++- deps/libff/libff/ff-decoder.h | 1 + deps/libff/libff/ff-video-decoder.c | 16 +++++++++++++--- plugins/obs-ffmpeg/obs-ffmpeg-source.c | 2 +- 5 files changed, 38 insertions(+), 10 deletions(-) diff --git a/deps/libff/libff/ff-audio-decoder.c b/deps/libff/libff/ff-audio-decoder.c index 88ad54c0e..07853c1a1 100644 --- a/deps/libff/libff/ff-audio-decoder.c +++ b/deps/libff/libff/ff-audio-decoder.c @@ -70,8 +70,14 @@ static int decode_frame(struct ff_decoder *decoder, int ret; while (true) { - ret = packet_queue_get(&decoder->packet_queue, packet, 1); - if (ret == FF_PACKET_FAIL) { + if (decoder->eof) + ret = packet_queue_get(&decoder->packet_queue, packet, 0); + else + ret = packet_queue_get(&decoder->packet_queue, packet, 1); + + if (ret == FF_PACKET_EMPTY) { + return 0; + } else if (ret == FF_PACKET_FAIL) { return -1; } @@ -134,8 +140,10 @@ static bool queue_frame(struct ff_decoder *decoder, AVFrame *frame, || queue_frame->frame->sample_rate != codec->sample_rate || queue_frame->frame->format != codec->sample_fmt); - if (queue_frame->frame != NULL) + if (queue_frame->frame != NULL) { + //FIXME: this shouldn't happen any more! av_frame_free(&queue_frame->frame); + } queue_frame->frame = av_frame_clone(frame); queue_frame->clock = ff_clock_retain(decoder->clock); @@ -157,10 +165,13 @@ void *ff_audio_decoder_thread(void *opaque_audio_decoder) struct ff_packet packet = {0}; bool frame_complete; AVFrame *frame = av_frame_alloc(); + int ret; while (!decoder->abort) { - if (decode_frame(decoder, &packet, frame, &frame_complete) - < 0) { + ret = decode_frame(decoder, &packet, frame, &frame_complete); + if (ret == 0) { + break; + } else if (ret < 0) { av_free_packet(&packet.base); continue; } @@ -184,5 +195,8 @@ void *ff_audio_decoder_thread(void *opaque_audio_decoder) ff_clock_release(&decoder->clock); av_frame_free(&frame); + + decoder->finished = true; + return NULL; } diff --git a/deps/libff/libff/ff-decoder.c b/deps/libff/libff/ff-decoder.c index cd12e61c9..a0e40881b 100644 --- a/deps/libff/libff/ff-decoder.c +++ b/deps/libff/libff/ff-decoder.c @@ -42,6 +42,7 @@ struct ff_decoder *ff_decoder_init(AVCodecContext *codec_context, decoder->codec->opaque = decoder; decoder->stream = stream; decoder->abort = false; + decoder->finished = false; decoder->packet_queue_size = packet_queue_size; if (!packet_queue_init(&decoder->packet_queue)) @@ -179,7 +180,7 @@ void ff_decoder_refresh(void *opaque) if (decoder && decoder->stream) { if (decoder->frame_queue.size == 0) { - if (!decoder->eof) { + if (!decoder->eof || !decoder->finished) { // We expected a frame, but there were none // available @@ -280,6 +281,8 @@ void ff_decoder_refresh(void *opaque) (int)(delay_until_next_wake * 1000 + 0.5L)); + av_frame_free(&frame->frame); + ff_circular_queue_advance_read(&decoder->frame_queue); } } else { diff --git a/deps/libff/libff/ff-decoder.h b/deps/libff/libff/ff-decoder.h index c0df5fc15..0050046e2 100644 --- a/deps/libff/libff/ff-decoder.h +++ b/deps/libff/libff/ff-decoder.h @@ -56,6 +56,7 @@ struct ff_decoder { bool first_frame; bool eof; bool abort; + bool finished; }; typedef struct ff_decoder ff_decoder_t; diff --git a/deps/libff/libff/ff-video-decoder.c b/deps/libff/libff/ff-video-decoder.c index 8366c0554..040d232cd 100644 --- a/deps/libff/libff/ff-video-decoder.c +++ b/deps/libff/libff/ff-video-decoder.c @@ -51,8 +51,11 @@ static bool queue_frame(struct ff_decoder *decoder, AVFrame *frame, || queue_frame->frame->height != codec->height || queue_frame->frame->format != codec->pix_fmt); - if (queue_frame->frame != NULL) + if (queue_frame->frame != NULL) { + // This shouldn't happen any more, the frames are freed in + // ff_decoder_refresh. av_frame_free(&queue_frame->frame); + } queue_frame->frame = av_frame_clone(frame); queue_frame->clock = ff_clock_retain(decoder->clock); @@ -78,8 +81,12 @@ void *ff_video_decoder_thread(void *opaque_video_decoder) bool key_frame; while (!decoder->abort) { - ret = packet_queue_get(&decoder->packet_queue, &packet, 1); - if (ret == FF_PACKET_FAIL) { + if (decoder->eof) + ret = packet_queue_get(&decoder->packet_queue, &packet, 0); + else + ret = packet_queue_get(&decoder->packet_queue, &packet, 1); + + if (ret == FF_PACKET_EMPTY || ret == FF_PACKET_FAIL) { // should we just use abort here? break; } @@ -137,5 +144,8 @@ void *ff_video_decoder_thread(void *opaque_video_decoder) ff_clock_release(&decoder->clock); av_frame_free(&frame); + + decoder->finished = true; + return NULL; } diff --git a/plugins/obs-ffmpeg/obs-ffmpeg-source.c b/plugins/obs-ffmpeg/obs-ffmpeg-source.c index 0e7aab1bd..c272ef5d8 100644 --- a/plugins/obs-ffmpeg/obs-ffmpeg-source.c +++ b/plugins/obs-ffmpeg/obs-ffmpeg-source.c @@ -269,7 +269,7 @@ static bool audio_frame(struct ff_frame *frame, void *opaque) uint64_t pts; // Media ended - if (frame == NULL) + if (frame == NULL || frame->frame == NULL) return true; pts = (uint64_t)(frame->pts * 1000000000.0L);