Set up packet duration correctly when encoding using FFmpeg (#2588).

It seems strange that this is necessary (maybe I'm missing some
other way that the packet duration is supposed to be set up).

Without this, when the mov muxer writes the trak tags it uses an
incorrect length value because the length value is calculated
from trk->end_pts, which in turn is calculated from the last-seen
pts + the duration of the last packet.  If that packet is marked
as length 0 the length of the track is 1 frame short, so the export
is missing a frame.

3 files changed, 46 insertions, 1 deletions

diff --git a/run/tests b/run/tests
index 3e648f202..b71cd6a66 100755
--- a/run/tests
+++ b/run/tests
@@ -3,7 +3,7 @@
 # e.g. --run_tests=foo
 set -e
 
-PRIVATE_GIT="906a1e49158e464b1c2887e6468e795aa575453d"
+PRIVATE_GIT="155d46e5c2a5a37c2269884b021c69b0d7d41f22"
 
 if [ "$1" == "--check" ]; then
 	shift 1
diff --git a/src/lib/ffmpeg_file_encoder.cc b/src/lib/ffmpeg_file_encoder.cc
index 32a0e0f86..f96a9458e 100644
--- a/src/lib/ffmpeg_file_encoder.cc
+++ b/src/lib/ffmpeg_file_encoder.cc
@@ -383,6 +383,7 @@ FFmpegFileEncoder::flush ()
 			throw EncodeError (N_("avcodec_receive_packet"), N_("FFmpegFileEncoder::flush"), r);
 		} else {
 			packet->stream_index = _video_stream_index;
+			packet->duration = _video_stream->time_base.den / _video_frame_rate;
 			av_interleaved_write_frame (_format_context, packet.get());
 		}
 
@@ -438,6 +439,7 @@ FFmpegFileEncoder::video (shared_ptr<PlayerVideo> video, DCPTime time)
 		throw EncodeError (N_("avcodec_receive_packet"), N_("FFmpegFileEncoder::video"), r);
 	} else if (r >= 0) {
 		packet->stream_index = _video_stream_index;
+		packet->duration = _video_stream->time_base.den / _video_frame_rate;
 		av_interleaved_write_frame (_format_context, packet.get());
 	}
 }
diff --git a/test/ffmpeg_encoder_test.cc b/test/ffmpeg_encoder_test.cc
index 8483330fd..dc57b473b 100644
--- a/test/ffmpeg_encoder_test.cc
+++ b/test/ffmpeg_encoder_test.cc
@@ -24,6 +24,7 @@
 #include "lib/config.h"
 #include "lib/constants.h"
 #include "lib/content_factory.h"
+#include "lib/cross.h"
 #include "lib/dcp_content.h"
 #include "lib/dcpomatic_log.h"
 #include "lib/ffmpeg_content.h"
@@ -37,11 +38,14 @@
 #include "lib/transcode_job.h"
 #include "lib/video_content.h"
 #include "test.h"
+#include <dcp/file.h>
+#include <dcp/raw_convert.h>
 #include <boost/test/unit_test.hpp>
 
 
 using std::make_shared;
 using std::string;
+using std::vector;
 using namespace dcpomatic;
 
 
@@ -510,3 +514,42 @@ BOOST_AUTO_TEST_CASE (ffmpeg_encoder_prores_regression_2)
 	cl.run();
 }
 
+
+BOOST_AUTO_TEST_CASE(ffmpeg_encoder_missing_frame_at_end)
+{
+	auto content = content_factory(TestPaths::private_data() / "1s1f.mov");
+	auto film = new_test_film2("ffmpeg_encoder_missing_frame_at_end", content);
+
+	boost::filesystem::path output("build/test/ffmpeg_encoder_missing_frame_at_end.mov");
+	boost::filesystem::path log("build/test/ffmpeg_encoder_missing_frame_at_end.log");
+
+	auto job = make_shared<TranscodeJob>(film, TranscodeJob::ChangedBehaviour::IGNORE);
+	FFmpegEncoder encoder(film, job, output, ExportFormat::PRORES_HQ, false, true, false, 23);
+	encoder.go();
+
+	run_ffprobe(output, log, false, "-show_frames -show_format -show_streams -select_streams v:0");
+
+	dcp::File read_log(log, "r");
+	BOOST_REQUIRE(read_log);
+
+	int nb_read_frames = 0;
+	int nb_frames = 0;
+	char buffer[256];
+
+	while (!read_log.eof()) {
+		read_log.gets(buffer, sizeof(buffer));
+		vector<string> parts;
+		boost::algorithm::split(parts, buffer, boost::is_any_of("="));
+		if (parts.size() == 2) {
+			if (parts[0] == "nb_read_frames") {
+				nb_read_frames = dcp::raw_convert<int>(parts[1]);
+			} else if (parts[0] == "nb_frames") {
+				nb_frames = dcp::raw_convert<int>(parts[1]);
+			}
+		}
+	}
+
+	BOOST_CHECK_EQUAL(nb_frames, 26);
+	BOOST_CHECK_EQUAL(nb_read_frames, 26);
+}
+