Fix incorrect extension on interop subtitle files (#2270).

diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc
index d64cfb81a4a6b553778fcef43bafa3aa0d7233fc..2f28ef9d4e3ebe706bdd9c39c84b6ed299feb5aa 100644 (file)
--- a/src/lib/reel_writer.cc
+++ b/src/lib/reel_writer.cc
@@ -484,7 +484,7 @@ maybe_add_text (
                if (auto interop = dynamic_pointer_cast<dcp::InteropSubtitleAsset>(asset)) {
                        auto directory = output_dcp / interop->id ();
                        boost::filesystem::create_directories (directory);
-                       interop->write (directory / subtitle_asset_filename(asset, reel_index, reel_count, content_summary));
+                       interop->write (directory / subtitle_asset_filename(asset, reel_index, reel_count, content_summary, ".xml"));
                        reel_asset = make_shared<Interop> (
                                interop,
                                dcp::Fraction(film->video_frame_rate(), 1),
@@ -499,7 +499,7 @@ maybe_add_text (
                        */
                        smpte->set_intrinsic_duration(picture_duration);
                        smpte->write (
-                               output_dcp / subtitle_asset_filename(asset, reel_index, reel_count, content_summary)
+                               output_dcp / subtitle_asset_filename(asset, reel_index, reel_count, content_summary, ".mxf")
                                );
                        reel_asset = make_shared<SMPTE> (
                                smpte,

diff --git a/src/lib/util.cc b/src/lib/util.cc
index 2c38257b4bafabf1447cc1824956e62da36e953d..79730d3472f7584e6aaccf95b2f46712e07f421d 100644 (file)
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -708,7 +708,7 @@ split_get_request (string url)
 
 static
 string
-asset_filename (shared_ptr<dcp::Asset> asset, string type, int reel_index, int reel_count, optional<string> summary)
+asset_filename (shared_ptr<dcp::Asset> asset, string type, int reel_index, int reel_count, optional<string> summary, string extension)
 {
        dcp::NameFormat::Map values;
        values['t'] = type;
@@ -717,35 +717,35 @@ asset_filename (shared_ptr<dcp::Asset> asset, string type, int reel_index, int r
        if (summary) {
                values['c'] = careful_string_filter(summary.get());
        }
-       return Config::instance()->dcp_asset_filename_format().get(values, "_" + asset->id() + ".mxf");
+       return Config::instance()->dcp_asset_filename_format().get(values, "_" + asset->id() + extension);
 }
 
 
 string
 video_asset_filename (shared_ptr<dcp::PictureAsset> asset, int reel_index, int reel_count, optional<string> summary)
 {
-       return asset_filename(asset, "j2c", reel_index, reel_count, summary);
+       return asset_filename(asset, "j2c", reel_index, reel_count, summary, ".mxf");
 }
 
 
 string
 audio_asset_filename (shared_ptr<dcp::SoundAsset> asset, int reel_index, int reel_count, optional<string> summary)
 {
-       return asset_filename(asset, "pcm", reel_index, reel_count, summary);
+       return asset_filename(asset, "pcm", reel_index, reel_count, summary, ".mxf");
 }
 
 
 string
-subtitle_asset_filename (shared_ptr<dcp::SubtitleAsset> asset, int reel_index, int reel_count, optional<string> summary)
+subtitle_asset_filename (shared_ptr<dcp::SubtitleAsset> asset, int reel_index, int reel_count, optional<string> summary, string extension)
 {
-       return asset_filename(asset, "sub", reel_index, reel_count, summary);
+       return asset_filename(asset, "sub", reel_index, reel_count, summary, extension);
 }
 
 
 string
 atmos_asset_filename (shared_ptr<dcp::AtmosAsset> asset, int reel_index, int reel_count, optional<string> summary)
 {
-       return asset_filename(asset, "atmos", reel_index, reel_count, summary);
+       return asset_filename(asset, "atmos", reel_index, reel_count, summary, ".mxf");
 }
 
 

diff --git a/src/lib/util.h b/src/lib/util.h
index cd5a1c2c56c2a0fa7c113ce4ad28f3ba5404a1c1..5878d5937ed3deeee8a65af5a2f8024f7cb3d414 100644 (file)
--- a/src/lib/util.h
+++ b/src/lib/util.h
@@ -108,7 +108,7 @@ extern void set_backtrace_file (boost::filesystem::path);
 extern std::map<std::string, std::string> split_get_request (std::string url);
 extern std::string video_asset_filename (std::shared_ptr<dcp::PictureAsset> asset, int reel_index, int reel_count, boost::optional<std::string> content_summary);
 extern std::string audio_asset_filename (std::shared_ptr<dcp::SoundAsset> asset, int reel_index, int reel_count, boost::optional<std::string> content_summary);
-extern std::string subtitle_asset_filename (std::shared_ptr<dcp::SubtitleAsset> asset, int reel_index, int reel_count, boost::optional<std::string> content_summary);
+extern std::string subtitle_asset_filename (std::shared_ptr<dcp::SubtitleAsset> asset, int reel_index, int reel_count, boost::optional<std::string> content_summary, std::string extension);
 extern std::string atmos_asset_filename (std::shared_ptr<dcp::AtmosAsset> asset, int reel_index, int reel_count, boost::optional<std::string> content_summary);
 extern float relaxed_string_to_float (std::string);
 extern std::string careful_string_filter (std::string);

diff --git a/test/file_extension_test.cc b/test/file_extension_test.cc
new file mode 100644 (file)
index 0000000..2c9020b
--- /dev/null
+++ b/test/file_extension_test.cc
@@ -0,0 +1,78 @@
+/*
+    Copyright (C) 2022 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    DCP-o-matic is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+
+#include "lib/content_factory.h"
+#include "lib/film.h"
+#include "test.h"
+#include <boost/test/unit_test.hpp>
+
+
+/* Sanity check to make sure that files in a DCP have the right extensions / names.
+ * This is mostly to catch a crazy mistake where Interop subtitle files suddenly got
+ * a MXF extension but no tests caught it (#2270).
+ */
+BOOST_AUTO_TEST_CASE (interop_file_extension_test)
+{
+       auto video = content_factory("test/data/flat_red.png").front();
+       auto audio = content_factory("test/data/sine_440.wav").front();
+       auto sub = content_factory("test/data/15s.srt").front();
+       auto film = new_test_film2("interop_file_extension_test", { video, audio, sub });
+       film->set_interop(true);
+
+       make_and_verify_dcp(
+               film, {
+                           dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME,
+                           dcp::VerificationNote::Code::MISSING_SUBTITLE_LANGUAGE,
+                           dcp::VerificationNote::Code::INVALID_STANDARD
+                       });
+
+       BOOST_REQUIRE(dcp_file(film, "ASSETMAP").extension() == "");
+       BOOST_REQUIRE(dcp_file(film, "VOLINDEX").extension() == "");
+       BOOST_REQUIRE(dcp_file(film, "cpl").extension() == ".xml");
+       BOOST_REQUIRE(dcp_file(film, "pkl").extension() == ".xml");
+       BOOST_REQUIRE(dcp_file(film, "j2c").extension() == ".mxf");
+       BOOST_REQUIRE(dcp_file(film, "pcm").extension() == ".mxf");
+       BOOST_REQUIRE(dcp_file(film, "sub").extension() == ".xml");
+}
+
+
+BOOST_AUTO_TEST_CASE (smpte_file_extension_test)
+{
+       auto video = content_factory("test/data/flat_red.png").front();
+       auto audio = content_factory("test/data/sine_440.wav").front();
+       auto sub = content_factory("test/data/15s.srt").front();
+       auto film = new_test_film2("smpte_file_extension_test", { video, audio, sub });
+       film->set_interop(false);
+
+       make_and_verify_dcp(
+               film, {
+                           dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME,
+                           dcp::VerificationNote::Code::MISSING_SUBTITLE_LANGUAGE
+                       });
+
+       BOOST_REQUIRE(dcp_file(film, "ASSETMAP").extension() == ".xml");
+       BOOST_REQUIRE(dcp_file(film, "VOLINDEX").extension() == ".xml");
+       BOOST_REQUIRE(dcp_file(film, "cpl").extension() == ".xml");
+       BOOST_REQUIRE(dcp_file(film, "pkl").extension() == ".xml");
+       BOOST_REQUIRE(dcp_file(film, "j2c").extension() == ".mxf");
+       BOOST_REQUIRE(dcp_file(film, "pcm").extension() == ".mxf");
+       BOOST_REQUIRE(dcp_file(film, "sub").extension() == ".mxf");
+}

diff --git a/test/test.cc b/test/test.cc
index 9260d568facce6a738b78f22bb182e18f4c5cd11..1059394731a9e21917683807f15ffbba64fe9e09 100644 (file)
--- a/test/test.cc
+++ b/test/test.cc
@@ -791,12 +791,12 @@ check_one_frame (boost::filesystem::path dcp_dir, int64_t index, boost::filesyst
 boost::filesystem::path
 dcp_file (shared_ptr<const Film> film, string prefix)
 {
-       auto i = boost::filesystem::directory_iterator (film->dir(film->dcp_name()));
-       while (i != boost::filesystem::directory_iterator() && !boost::algorithm::starts_with (i->path().leaf().string(), prefix)) {
+       auto i = boost::filesystem::recursive_directory_iterator(film->dir(film->dcp_name()));
+       while (i != boost::filesystem::recursive_directory_iterator() && !boost::algorithm::starts_with(i->path().leaf().string(), prefix)) {
                ++i;
        }
 
-       BOOST_REQUIRE (i != boost::filesystem::directory_iterator());
+       BOOST_REQUIRE_MESSAGE(i != boost::filesystem::recursive_directory_iterator(), "Could not find file with prefix " << prefix);
        return i->path();
 }
 

diff --git a/test/wscript b/test/wscript
index 88ff8c6bc65c5f15d8d156205c2fbc4ee80260a8..e05f85a582fcd6ef0499c03a5aaf065ba7f77eaa 100644 (file)
--- a/test/wscript
+++ b/test/wscript
@@ -76,6 +76,7 @@ def build(bld):
                  empty_caption_test.cc
                  empty_test.cc
                  encryption_test.cc
+                 file_extension_test.cc
                  ffmpeg_audio_only_test.cc
                  ffmpeg_audio_test.cc
                  ffmpeg_dcp_test.cc