Merge remote-tracking branch 'origin/main' into v2.17.x

6 files changed, 68 insertions, 37 deletions

diff --git a/run/tests b/run/tests
index 4e38dbfce..8395b09d4 100755
--- a/run/tests
+++ b/run/tests
@@ -3,7 +3,7 @@
 # e.g. --run_tests=foo
 set -e
 
-PRIVATE_GIT="881c48805e352dfe150993814757ca974282be18"
+PRIVATE_GIT="65c6d0cf0ec188e4d53fc8c9d61958c7921219d6"
 
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 source $DIR/environment
diff --git a/src/lib/audio_stream.h b/src/lib/audio_stream.h
index cf874242f..b125eb8c0 100644
--- a/src/lib/audio_stream.h
+++ b/src/lib/audio_stream.h
@@ -54,6 +54,11 @@ public:
 		return _length;
 	}
 
+	void set_length(Frame length) {
+		boost::mutex::scoped_lock lm (_mutex);
+		_length = length;
+	}
+
 	int channels () const;
 	boost::optional<int> bit_depth() const;
 
diff --git a/src/lib/ffmpeg.cc b/src/lib/ffmpeg.cc
index d9df232df..550e045b3 100644
--- a/src/lib/ffmpeg.cc
+++ b/src/lib/ffmpeg.cc
@@ -326,6 +326,8 @@ FFmpeg::pts_offset (vector<shared_ptr<FFmpegAudioStream>> audio_streams, optiona
 		}
 	}
 
+	DCPOMATIC_ASSERT(po != ContentTime::min());
+
 	/* If the 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.
diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc
index d173c6bb6..8fa49770b 100644
--- a/src/lib/ffmpeg_examiner.cc
+++ b/src/lib/ffmpeg_examiner.cc
@@ -62,10 +62,8 @@ static const int PULLDOWN_CHECK_FRAMES = 16;
 /** @param job job that the examiner is operating in, or 0 */
 FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Job> job)
 	: FFmpeg (c)
-	, _video_length (0)
-	, _need_video_length (false)
-	, _pulldown (false)
 {
+	_need_length = _format_context->duration == AV_NOPTS_VALUE;
 	/* Find audio and subtitle streams */
 
 	for (uint32_t i = 0; i < _format_context->nb_streams; ++i) {
@@ -73,7 +71,6 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Jo
 		auto codec = _codec_context[i] ? _codec_context[i]->codec : nullptr;
 		if (s->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && codec) {
 
-			DCPOMATIC_ASSERT (_format_context->duration != AV_NOPTS_VALUE);
 			DCPOMATIC_ASSERT (codec->name);
 
 			_audio_streams.push_back (
@@ -82,7 +79,7 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Jo
 					codec->name,
 					s->id,
 					s->codecpar->sample_rate,
-					llrint ((double(_format_context->duration) / AV_TIME_BASE) * s->codecpar->sample_rate),
+					_need_length ? 0 : rint ((double(_format_context->duration) / AV_TIME_BASE) * s->codecpar->sample_rate),
 					s->codecpar->ch_layout.nb_channels,
 					s->codecpar->bits_per_raw_sample ? s->codecpar->bits_per_raw_sample : s->codecpar->bits_per_coded_sample
 					)
@@ -95,13 +92,10 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Jo
 
 	if (has_video ()) {
 		/* See if the header has duration information in it */
-		_need_video_length = _format_context->duration == AV_NOPTS_VALUE;
-		if (!_need_video_length) {
-			_video_length = llrint ((double (_format_context->duration) / AV_TIME_BASE) * video_frame_rate().get());
-		}
+		_video_length = _need_length ? 0 : llrint((double (_format_context->duration) / AV_TIME_BASE) * video_frame_rate().get());
 	}
 
-	if (job && _need_video_length) {
+	if (job && _need_length) {
 		job->sub (_("Finding length"));
 	}
 
@@ -109,6 +103,7 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Jo
 	 *   - the first video.
 	 *   - the first audio for each stream.
 	 *   - the top-field-first and repeat-first-frame values ("temporal_reference") for the first PULLDOWN_CHECK_FRAMES video frames.
+	 * or forever if _need_length is true.
 	 */
 
 	int64_t const len = _file_group.length ();
@@ -117,6 +112,8 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Jo
 	 * and a string seems a reasonably neat way to do that.
 	 */
 	string temporal_reference;
+	bool carry_on_video = false;
+	std::vector<bool> carry_on_audio(_audio_streams.size());
 	while (true) {
 		auto packet = av_packet_alloc ();
 		DCPOMATIC_ASSERT (packet);
@@ -136,26 +133,37 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr<const FFmpegContent> c, shared_ptr<Jo
 
 		auto context = _codec_context[packet->stream_index];
 
-		if (_video_stream && packet->stream_index == _video_stream.get()) {
-			video_packet (context, temporal_reference, packet);
-		}
-
-		bool got_all_audio = true;
-
+		boost::optional<size_t> audio_stream_index;
 		for (size_t i = 0; i < _audio_streams.size(); ++i) {
 			if (_audio_streams[i]->uses_index(_format_context, packet->stream_index)) {
-				audio_packet (context, _audio_streams[i], packet);
+				audio_stream_index = i;
 			}
-			if (!_audio_streams[i]->first_audio) {
-				got_all_audio = false;
+		}
+
+		bool const video = _video_stream && packet->stream_index == *_video_stream;
+
+		if (!video && !audio_stream_index) {
+			av_packet_free(&packet);
+			continue;
+		}
+
+		if (video) {
+			carry_on_video = video_packet(context, temporal_reference, packet);
+		}
+
+		if (audio_stream_index) {
+			if (audio_packet(context, _audio_streams[*audio_stream_index], packet)) {
+				carry_on_audio[*audio_stream_index] = true;
 			}
 		}
 
 		av_packet_free (&packet);
 
-		if (got_all_audio && (!_video_stream || (_first_video && temporal_reference.size() >= (PULLDOWN_CHECK_FRAMES * 2)))) {
-			/* All done */
-			break;
+		if (!carry_on_video) {
+			if (std::find(carry_on_audio.begin(), carry_on_audio.end(), true) == carry_on_audio.end()) {
+				/* All done */
+				break;
+			}
 		}
 	}
 
@@ -210,7 +218,7 @@ FFmpegExaminer::video_packet (AVCodecContext* context, string& temporal_referenc
 {
 	DCPOMATIC_ASSERT (_video_stream);
 
-	if (_first_video && !_need_video_length && temporal_reference.size() >= (PULLDOWN_CHECK_FRAMES * 2)) {
+	if (_first_video && !_need_length && temporal_reference.size() >= (PULLDOWN_CHECK_FRAMES * 2)) {
 		return false;
 	}
 
@@ -237,11 +245,11 @@ FFmpegExaminer::video_packet (AVCodecContext* context, string& temporal_referenc
 	if (!_first_video) {
 		_first_video = frame_time (_video_frame, _format_context->streams[_video_stream.get()]);
 	}
-	if (_need_video_length) {
-		_video_length = frame_time (
+	if (_need_length) {
+		_video_length = frame_time(
 			_video_frame,
 			_format_context->streams[_video_stream.get()]
-			).get_value_or (ContentTime ()).frames_round (video_frame_rate().get ());
+			).get_value_or({}).frames_round(video_frame_rate().get()) + 1;
 	}
 	if (temporal_reference.size() < (PULLDOWN_CHECK_FRAMES * 2)) {
 		temporal_reference += ((_video_frame->flags & AV_FRAME_FLAG_TOP_FIELD_FIRST) ? "T" : "B");
@@ -252,26 +260,34 @@ FFmpegExaminer::video_packet (AVCodecContext* context, string& temporal_referenc
 }
 
 
-void
+bool
 FFmpegExaminer::audio_packet (AVCodecContext* context, shared_ptr<FFmpegAudioStream> stream, AVPacket* packet)
 {
-	if (stream->first_audio) {
-		return;
+	if (stream->first_audio && !_need_length) {
+		return false;
 	}
 
 	int r = avcodec_send_packet (context, packet);
 	if (r < 0) {
 		LOG_WARNING("avcodec_send_packet returned %1 for an audio packet", r);
-		return;
+		return false;
 	}
 
 	auto frame = audio_frame (stream);
 
 	if (avcodec_receive_frame (context, frame) < 0) {
-		return;
+		return false;
 	}
 
-	stream->first_audio = frame_time (frame, stream->stream(_format_context));
+	if (!stream->first_audio) {
+		stream->first_audio = frame_time(frame, stream->stream(_format_context));
+	}
+
+	if (_need_length) {
+		stream->set_length(frame_time(frame, stream->stream(_format_context)).get_value_or({}).frames_round(stream->frame_rate()) + frame->nb_samples);
+	}
+
+	return true;
 }
 
 
diff --git a/src/lib/ffmpeg_examiner.h b/src/lib/ffmpeg_examiner.h
index 45313ec18..ad64f349d 100644
--- a/src/lib/ffmpeg_examiner.h
+++ b/src/lib/ffmpeg_examiner.h
@@ -89,7 +89,7 @@ public:
 
 private:
 	bool video_packet (AVCodecContext* context, std::string& temporal_reference, AVPacket* packet);
-	void audio_packet (AVCodecContext* context, std::shared_ptr<FFmpegAudioStream>, AVPacket* packet);
+	bool audio_packet (AVCodecContext* context, std::shared_ptr<FFmpegAudioStream>, AVPacket* packet);
 
 	std::string stream_name (AVStream* s) const;
 	std::string subtitle_stream_name (AVStream* s) const;
@@ -101,11 +101,11 @@ private:
 	/** Video length, either obtained from the header or derived by running
 	 *  through the whole file.
 	 */
-	Frame _video_length;
-	bool _need_video_length;
+	Frame _video_length = 0;
+	bool _need_length = false;
 
 	boost::optional<double> _rotation;
-	bool _pulldown;
+	bool _pulldown = false;
 
 	struct SubtitleStart
 	{
diff --git a/test/ffmpeg_audio_test.cc b/test/ffmpeg_audio_test.cc
index 01bc38da3..c973bc362 100644
--- a/test/ffmpeg_audio_test.cc
+++ b/test/ffmpeg_audio_test.cc
@@ -136,3 +136,11 @@ BOOST_AUTO_TEST_CASE (ffmpeg_audio_test4)
 	BOOST_CHECK_NO_THROW (while (!player->pass()) {});
 }
 
+
+
+BOOST_AUTO_TEST_CASE(no_audio_length_in_header)
+{
+	auto content = content_factory(TestPaths::private_data() / "10-seconds.thd");
+	auto film = new_test_film2("no_audio_length_in_header", content);
+	BOOST_CHECK(content[0]->full_length(film) == dcpomatic::DCPTime::from_seconds(10));
+}