From: Carl Hetherington Date: Tue, 7 Jun 2022 10:36:40 +0000 (+0200) Subject: Fix incorrect extension on interop subtitle files (#2270). X-Git-Tag: v2.16.14~35 X-Git-Url: https://main.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=6a3c03c5eed3cab8fdfdb04fcbaf6cabe8c715e9 Fix incorrect extension on interop subtitle files (#2270). --- diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc index d64cfb81a..2f28ef9d4 100644 --- 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(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, 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, diff --git a/src/lib/util.cc b/src/lib/util.cc index 2c38257b4..79730d347 100644 --- 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 asset, string type, int reel_index, int reel_count, optional summary) +asset_filename (shared_ptr asset, string type, int reel_index, int reel_count, optional summary, string extension) { dcp::NameFormat::Map values; values['t'] = type; @@ -717,35 +717,35 @@ asset_filename (shared_ptr 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 asset, int reel_index, int reel_count, optional 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 asset, int reel_index, int reel_count, optional 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 asset, int reel_index, int reel_count, optional summary) +subtitle_asset_filename (shared_ptr asset, int reel_index, int reel_count, optional 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 asset, int reel_index, int reel_count, optional 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 cd5a1c2c5..5878d5937 100644 --- 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 split_get_request (std::string url); extern std::string video_asset_filename (std::shared_ptr asset, int reel_index, int reel_count, boost::optional content_summary); extern std::string audio_asset_filename (std::shared_ptr asset, int reel_index, int reel_count, boost::optional content_summary); -extern std::string subtitle_asset_filename (std::shared_ptr asset, int reel_index, int reel_count, boost::optional content_summary); +extern std::string subtitle_asset_filename (std::shared_ptr asset, int reel_index, int reel_count, boost::optional content_summary, std::string extension); extern std::string atmos_asset_filename (std::shared_ptr asset, int reel_index, int reel_count, boost::optional 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 index 000000000..2c9020b6c --- /dev/null +++ b/test/file_extension_test.cc @@ -0,0 +1,78 @@ +/* + Copyright (C) 2022 Carl Hetherington + + 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 . + +*/ + + +#include "lib/content_factory.h" +#include "lib/film.h" +#include "test.h" +#include + + +/* 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 9260d568f..105939473 100644 --- 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 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 88ff8c6bc..e05f85a58 100644 --- 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