X-Git-Url: https://main.carlh.net/gitweb/?a=blobdiff_plain;f=src%2Flib%2Fffmpeg_decoder.cc;h=2d12ef0a007673d612babd8919f0cedb48d5ba0c;hb=ec72668d88935635a01f602f0aa33bfa5a5c426e;hp=0e15880024f9d0a1eb3ebbbcb9246a6509b3ae79;hpb=ebd735b9ccbf4b959029da0a6cc33bb01fb3bcdd;p=dcpomatic.git

diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index 0e1588002..2d12ef0a0 100644
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -23,7 +23,6 @@
 
 #include <stdexcept>
 #include <vector>
-#include <sstream>
 #include <iomanip>
 #include <iostream>
 #include <stdint.h>
@@ -51,12 +50,12 @@ extern "C" {
 
 #define LOG_GENERAL(...) _video_content->film()->log()->log (String::compose (__VA_ARGS__), Log::TYPE_GENERAL);
 #define LOG_ERROR(...) _video_content->film()->log()->log (String::compose (__VA_ARGS__), Log::TYPE_ERROR);
-#define LOG_WARNING(...) _video_content->film()->log()->log (__VA_ARGS__, Log::TYPE_WARNING);
+#define LOG_WARNING_NC(...) _video_content->film()->log()->log (__VA_ARGS__, Log::TYPE_WARNING);
+#define LOG_WARNING(...) _video_content->film()->log()->log (String::compose (__VA_ARGS__), Log::TYPE_WARNING);
 
 using std::cout;
 using std::string;
 using std::vector;
-using std::stringstream;
 using std::list;
 using std::min;
 using std::pair;
@@ -98,6 +97,14 @@ FFmpegDecoder::FFmpegDecoder (shared_ptr<const FFmpegContent> c, shared_ptr<Log>
 		_pts_offset = - c->audio_stream()->first_audio.get();
 	}
 
+	/* If _pts_offset is positive we would be pushing things from a -ve PTS to be played.
+	   I don't think we ever want to do that, as it seems things at -ve PTS are not meant
+	   to be seen (use for alignment bars etc.); see mantis #418.
+	*/
+	if (_pts_offset > ContentTime ()) {
+		_pts_offset = ContentTime ();
+	}
+
 	/* Now adjust both so that the video pts starts on a frame */
 	if (have_video && have_audio) {
 		ContentTime first_video = c->first_video().get() + _pts_offset;
@@ -129,14 +136,18 @@ FFmpegDecoder::pass ()
 {
 	int r = av_read_frame (_format_context, &_packet);
 
-	if (r < 0) {
+	/* AVERROR_INVALIDDATA can apparently be returned sometimes even when av_read_frame
+	   has pretty-much succeeded (and hence generated data which should be processed).
+	   Hence it makes sense to continue here in that case.
+	*/
+	if (r < 0 && r != AVERROR_INVALIDDATA) {
 		if (r != AVERROR_EOF) {
 			/* Maybe we should fail here, but for now we'll just finish off instead */
 			char buf[256];
 			av_strerror (r, buf, sizeof(buf));
 			LOG_ERROR (N_("error on av_read_frame (%1) (%2)"), buf, r);
 		}
-
+		
 		flush ();
 		return true;
 	}
@@ -161,12 +172,12 @@ FFmpegDecoder::pass ()
 shared_ptr<AudioBuffers>
 FFmpegDecoder::deinterleave_audio (uint8_t** data, int size)
 {
-	assert (_ffmpeg_content->audio_channels());
-	assert (bytes_per_audio_sample());
+	DCPOMATIC_ASSERT (_ffmpeg_content->audio_channels());
+	DCPOMATIC_ASSERT (bytes_per_audio_sample());
 
 	/* Deinterleave and convert to float */
 
-	assert ((size % (bytes_per_audio_sample() * _ffmpeg_content->audio_channels())) == 0);
+	DCPOMATIC_ASSERT ((size % (bytes_per_audio_sample() * _ffmpeg_content->audio_channels())) == 0);
 
 	int const total_samples = size / bytes_per_audio_sample();
 	int const frames = total_samples / _ffmpeg_content->audio_channels();
@@ -296,9 +307,6 @@ FFmpegDecoder::seek (ContentTime time, bool accurate)
 
 	ContentTime pre_roll = accurate ? ContentTime::from_seconds (2) : ContentTime (0);
 	time -= pre_roll;
-	if (time < ContentTime (0)) {
-		time = ContentTime (0);
-	}
 
 	/* XXX: it seems debatable whether PTS should be used here...
 	   http://www.mjbshaw.com/2012/04/seeking-in-ffmpeg-know-your-timestamp.html
@@ -313,12 +321,6 @@ FFmpegDecoder::seek (ContentTime time, bool accurate)
 			);
 	}
 
-	/* Ridiculous empirical hack */
-	s--;
-	if (s < 0) {
-		s = 0;
-	}
-
 	av_seek_frame (_format_context, _video_stream, s, 0);
 
 	avcodec_flush_buffers (video_codec_context());
@@ -342,11 +344,21 @@ FFmpegDecoder::decode_audio_packet ()
 	while (copy_packet.size > 0) {
 
 		int frame_finished;
-		int const decode_result = avcodec_decode_audio4 (audio_codec_context(), _frame, &frame_finished, &copy_packet);
-
+		int decode_result = avcodec_decode_audio4 (audio_codec_context(), _frame, &frame_finished, &copy_packet);
 		if (decode_result < 0) {
-			LOG_ERROR ("avcodec_decode_audio4 failed (%1)", decode_result);
-			return;
+			/* avcodec_decode_audio4 can sometimes return an error even though it has decoded
+			   some valid data; for example dca_subframe_footer can return AVERROR_INVALIDDATA
+			   if it overreads the auxiliary data.  ffplay carries on if frame_finished is true,
+			   even in the face of such an error, so I think we should too.
+
+			   Returning from the method here caused mantis #352.
+			*/
+			LOG_WARNING ("avcodec_decode_audio4 failed (%1)", decode_result);
+
+			/* Fudge decode_result so that we come out of the while loop when
+			   we've processed this data.
+			*/
+			decode_result = copy_packet.size;
 		}
 
 		if (frame_finished) {
@@ -401,11 +413,11 @@ FFmpegDecoder::decode_video_packet ()
 		if (i->second != AV_NOPTS_VALUE) {
 			double const pts = i->second * av_q2d (_format_context->streams[_video_stream]->time_base) + _pts_offset.seconds ();
 			video (
-				shared_ptr<ImageProxy> (new RawImageProxy (image, _video_content->film()->log())),
+				shared_ptr<ImageProxy> (new RawImageProxy (image)),
 				rint (pts * _ffmpeg_content->video_frame_rate ())
 				);
 		} else {
-			LOG_WARNING ("Dropping frame without PTS");
+			LOG_WARNING_NC ("Dropping frame without PTS");
 		}
 	}