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.

diff --git a/run/tests b/run/tests
index 3e648f202e9b2975708ee8a7ee05f8bb9bea3138..b71cd6a66ccf92b3f55e8acb94e2d2476de0efd4 100755 (executable)
--- 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 32a0e0f8620f3741ab09bd6c7456305541692130..f96a9458ec132c92993a138d39bf8e38094a0b33 100644 (file)
--- 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 8483330fd188a1d9887c74dacc73061053a21a89..dc57b473b48a3174ea080b8e2f3df545b680d260 100644 (file)
--- 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);
+}
+