Improve OpenFileError so that it doesn't say "opening for read"

in one case where it should say "opening for read/write".

Also add some unit tests for ReelWriter.

diff --git a/src/lib/exceptions.cc b/src/lib/exceptions.cc
index c86f98da2e5912a90300484252c44ff8893a321d..99d4c3979a602dc5e1a84c0673e29c10c1e9d4e5 100644 (file)
--- a/src/lib/exceptions.cc
+++ b/src/lib/exceptions.cc
@@ -27,10 +27,11 @@ using std::string;
 using std::runtime_error;
 
 /** @param f File that we were trying to open */
-OpenFileError::OpenFileError (boost::filesystem::path f, int error, bool reading)
+OpenFileError::OpenFileError (boost::filesystem::path f, int error, Mode mode)
        : FileError (
                String::compose (
-                       reading ? _("could not open file %1 for reading (%2)") : _("could not open file %1 for writing (%2)"),
+                       mode == READ_WRITE ? _("could not open file %1 for read/write (%2)") :
+                       (mode == READ ? _("could not open file %1 for read (%2)") : _("could not open file %1 for write (%2)")),
                        f.string(),
                        error),
                f

diff --git a/src/lib/exceptions.h b/src/lib/exceptions.h
index f6b3bd902dd7a0cc8df5201ded9c08122bdd6989..ddd1e36035f9dfb6a5e71827a2b47f8c978da3f0 100644 (file)
--- a/src/lib/exceptions.h
+++ b/src/lib/exceptions.h
@@ -103,11 +103,17 @@ public:
 class OpenFileError : public FileError
 {
 public:
+       enum Mode {
+               READ,
+               WRITE,
+               READ_WRITE
+       };
+
        /** @param f File that we were trying to open.
         *  @param error Code of error that occurred.
-        *  @param reading true if we were opening to read, false if opening to write.
+        *  @param mode Mode that we tried to open the file in.
         */
-       OpenFileError (boost::filesystem::path f, int error, bool reading);
+       OpenFileError (boost::filesystem::path f, int error, Mode mode);
 };
 
 /** @class ReadFileError.

diff --git a/src/lib/ffmpeg.cc b/src/lib/ffmpeg.cc
index adc5c224c6f4e8ea408287de3a7360f3cb3e25fc..0668f9f37b381707b726cee11086ec4511845bca 100644 (file)
--- a/src/lib/ffmpeg.cc
+++ b/src/lib/ffmpeg.cc
@@ -134,7 +134,7 @@ FFmpeg::setup_general ()
 
        int e = avformat_open_input (&_format_context, 0, 0, &options);
        if (e < 0) {
-               throw OpenFileError (_ffmpeg_content->path(0).string(), e, true);
+               throw OpenFileError (_ffmpeg_content->path(0).string(), e, OpenFileError::READ);
        }
 
        if (avformat_find_stream_info (_format_context, 0) < 0) {

diff --git a/src/lib/ffmpeg_image_proxy.cc b/src/lib/ffmpeg_image_proxy.cc
index fa6cb288d28637ece3f55921cf69edb19fe1065e..9c91d1d87d7d6c60eb883ee346acbb14876bf5ee 100644 (file)
--- a/src/lib/ffmpeg_image_proxy.cc
+++ b/src/lib/ffmpeg_image_proxy.cc
@@ -147,7 +147,7 @@ FFmpegImageProxy::image (optional<dcp::Size>) const
        }
        if (e < 0) {
                if (_path) {
-                       throw OpenFileError (_path->string(), e, true);
+                       throw OpenFileError (_path->string(), e, OpenFileError::READ);
                } else {
                        boost::throw_exception(DecodeError(String::compose(_("Could not decode image (%1)"), e)));
                }

diff --git a/src/lib/file_group.cc b/src/lib/file_group.cc
index 942eb435d87f081e38ef5e48bf53ea0e4dd3cdea..3e8a7b79c43177a8d775889ab91d08855bb0e77e 100644 (file)
--- a/src/lib/file_group.cc
+++ b/src/lib/file_group.cc
@@ -93,7 +93,7 @@ FileGroup::ensure_open_path (size_t p) const
        _current_path = p;
        _current_file = fopen_boost (_paths[_current_path], "rb");
        if (_current_file == 0) {
-               throw OpenFileError (_paths[_current_path], errno, true);
+               throw OpenFileError (_paths[_current_path], errno, OpenFileError::READ);
        }
 }
 

diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc
index 69709fa0de5d0dbfc899f49547c0059805f273ba..2631489028f22875aa26f5620f036277f49314df 100644 (file)
--- a/src/lib/reel_writer.cc
+++ b/src/lib/reel_writer.cc
@@ -64,6 +64,7 @@ using namespace dcpomatic;
 
 int const ReelWriter::_info_size = 48;
 
+/** @param job Related job, or 0 */
 ReelWriter::ReelWriter (
        shared_ptr<const Film> film, DCPTimePeriod period, shared_ptr<Job> job, int reel_index, int reel_count, optional<string> content_summary
        )
@@ -99,7 +100,9 @@ ReelWriter::ReelWriter (
                _film->internal_video_asset_dir() / _film->internal_video_asset_filename(_period)
                );
 
-       job->sub (_("Checking existing image data"));
+       if (job) {
+               job->sub (_("Checking existing image data"));
+       }
        _first_nonexistant_frame = check_existing_picture_asset ();
 
        _picture_asset_writer = _picture_asset->start_write (
@@ -141,8 +144,9 @@ ReelWriter::write_frame_info (Frame frame, Eyes eyes, dcp::FrameInfo info) const
        } else {
                file = fopen_boost (info_file, "wb");
        }
+
        if (!file) {
-               throw OpenFileError (info_file, errno, read);
+               throw OpenFileError (info_file, errno, read ? OpenFileError::READ_WRITE : OpenFileError::WRITE);
        }
        dcpomatic_fseek (file, frame_info_position (frame, eyes), SEEK_SET);
        checked_fwrite (&info.offset, sizeof (info.offset), file, info_file);

diff --git a/src/lib/reel_writer.h b/src/lib/reel_writer.h
index fff2e0b9ea7568d646b4ef0b18b86e5104757799..b96bcfc68de2b19d8f352a123c841354056c71e3 100644 (file)
--- a/src/lib/reel_writer.h
+++ b/src/lib/reel_writer.h
@@ -33,6 +33,7 @@ namespace dcpomatic {
 class Film;
 class Job;
 class AudioBuffers;
+struct write_frame_info_test;
 
 namespace dcp {
        class MonoPictureAsset;
@@ -92,6 +93,8 @@ public:
 
 private:
 
+       friend struct ::write_frame_info_test;
+
        void write_frame_info (Frame frame, Eyes eyes, dcp::FrameInfo info) const;
        long frame_info_position (Frame frame, Eyes eyes) const;
        Frame check_existing_picture_asset ();

diff --git a/src/lib/util.cc b/src/lib/util.cc
index fee4a3c26aab11ac4bf094d5977d6d9c27e18e9a..2dedc32c152efc60161722d411c7ef0511b018df 100644 (file)
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -473,7 +473,7 @@ digest_head_tail (vector<boost::filesystem::path> files, boost::uintmax_t size)
        while (i < int64_t (files.size()) && to_do > 0) {
                FILE* f = fopen_boost (files[i], "rb");
                if (!f) {
-                       throw OpenFileError (files[i].string(), errno, true);
+                       throw OpenFileError (files[i].string(), errno, OpenFileError::READ);
                }
 
                boost::uintmax_t this_time = min (to_do, boost::filesystem::file_size (files[i]));
@@ -493,7 +493,7 @@ digest_head_tail (vector<boost::filesystem::path> files, boost::uintmax_t size)
        while (i >= 0 && to_do > 0) {
                FILE* f = fopen_boost (files[i], "rb");
                if (!f) {
-                       throw OpenFileError (files[i].string(), errno, true);
+                       throw OpenFileError (files[i].string(), errno, OpenFileError::READ);
                }
 
                boost::uintmax_t this_time = min (to_do, boost::filesystem::file_size (files[i]));

diff --git a/src/lib/writer.cc b/src/lib/writer.cc
index 3fc01571e559756fc9506bc818a4e07a1e09d258..5b24d7491b85751734a01bc71887351c6aac8959 100644 (file)
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -586,7 +586,7 @@ Writer::write_cover_sheet ()
        boost::filesystem::path const cover = _film->file ("COVER_SHEET.txt");
        FILE* f = fopen_boost (cover, "w");
        if (!f) {
-               throw OpenFileError (cover, errno, false);
+               throw OpenFileError (cover, errno, OpenFileError::WRITE);
        }
 
        string text = Config::instance()->cover_sheet ();

diff --git a/src/wx/config_dialog.cc b/src/wx/config_dialog.cc
index a6b299bcb6dd6c976baf59bdfda754fdef475d1f..98a1a1a8e231c29505ab2385fc41a4e892aede45 100644 (file)
--- a/src/wx/config_dialog.cc
+++ b/src/wx/config_dialog.cc
@@ -579,7 +579,7 @@ CertificateChainEditor::export_certificate ()
                boost::filesystem::path path (wx_to_std(d->GetPath()));
                FILE* f = fopen_boost (path, "w");
                if (!f) {
-                       throw OpenFileError (path, errno, false);
+                       throw OpenFileError (path, errno, OpenFileError::WRITE);
                }
 
                string const s = j->certificate (true);
@@ -601,7 +601,7 @@ CertificateChainEditor::export_chain ()
                boost::filesystem::path path (wx_to_std(d->GetPath()));
                FILE* f = fopen_boost (path, "w");
                if (!f) {
-                       throw OpenFileError (path, errno, false);
+                       throw OpenFileError (path, errno, OpenFileError::WRITE);
                }
 
                string const s = _get()->chain();
@@ -775,7 +775,7 @@ CertificateChainEditor::export_private_key ()
                boost::filesystem::path path (wx_to_std(d->GetPath()));
                FILE* f = fopen_boost (path, "w");
                if (!f) {
-                       throw OpenFileError (path, errno, false);
+                       throw OpenFileError (path, errno, OpenFileError::WRITE);
                }
 
                string const s = _get()->key().get ();
@@ -868,7 +868,7 @@ KeysPage::export_decryption_chain_and_key ()
                boost::filesystem::path path (wx_to_std(d->GetPath()));
                FILE* f = fopen_boost (path, "w");
                if (!f) {
-                       throw OpenFileError (path, errno, false);
+                       throw OpenFileError (path, errno, OpenFileError::WRITE);
                }
 
                string const chain = Config::instance()->decryption_chain()->chain();
@@ -903,7 +903,7 @@ KeysPage::import_decryption_chain_and_key ()
 
                FILE* f = fopen_boost (wx_to_std (d->GetPath ()), "r");
                if (!f) {
-                       throw OpenFileError (wx_to_std (d->GetPath ()), errno, false);
+                       throw OpenFileError (wx_to_std (d->GetPath ()), errno, OpenFileError::WRITE);
                }
 
                string current;
@@ -955,7 +955,7 @@ KeysPage::export_decryption_certificate ()
                boost::filesystem::path path (wx_to_std(d->GetPath()));
                FILE* f = fopen_boost (path, "w");
                if (!f) {
-                       throw OpenFileError (path, errno, false);
+                       throw OpenFileError (path, errno, OpenFileError::WRITE);
                }
 
                string const s = Config::instance()->decryption_chain()->leaf().certificate (true);

diff --git a/test/reel_writer_test.cc b/test/reel_writer_test.cc
new file mode 100644 (file)
index 0000000..db63ca8
--- /dev/null
+++ b/test/reel_writer_test.cc
@@ -0,0 +1,88 @@
+/*
+    Copyright (C) 2019 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/>.
+
+*/
+
+/** @file  test/reel_writer_test.cc
+ *  @brief Test ReelWriter class.
+ *  @ingroup selfcontained
+ */
+
+#include "lib/reel_writer.h"
+#include "lib/film.h"
+#include "lib/cross.h"
+#include "test.h"
+#include <boost/test/unit_test.hpp>
+
+using std::string;
+using boost::shared_ptr;
+using boost::optional;
+
+static bool equal (dcp::FrameInfo a, ReelWriter const & writer, boost::filesystem::path file, Frame frame, Eyes eyes)
+{
+       FILE* f = fopen_boost(file, "rb");
+       BOOST_REQUIRE (f);
+       dcp::FrameInfo b = writer.read_frame_info(f, frame, eyes);
+       bool const r = a.offset == b.offset && a.size == b.size && a.hash == b.hash;
+       fclose (f);
+       return r;
+}
+
+BOOST_AUTO_TEST_CASE (write_frame_info_test)
+{
+       shared_ptr<Film> film = new_test_film2 ("write_frame_info_test");
+       dcpomatic::DCPTimePeriod const period (dcpomatic::DCPTime(0), dcpomatic::DCPTime(96000));
+       ReelWriter writer (film, period, shared_ptr<Job>(), 0, 1, optional<string>());
+
+       /* Write the first one */
+
+       boost::filesystem::path file = film->info_file (period);
+       BOOST_CHECK (!boost::filesystem::exists(file));
+       dcp::FrameInfo info1(0, 123, "12345678901234567890123456789012");
+       writer.write_frame_info (0, EYES_LEFT, info1);
+       BOOST_CHECK (boost::filesystem::exists(file));
+
+       BOOST_CHECK (equal(info1, writer, file, 0, EYES_LEFT));
+
+       /* Write some more */
+
+       dcp::FrameInfo info2(596, 14921, "123acb789f1234ae782012n456339522");
+       writer.write_frame_info (5, EYES_RIGHT, info2);
+       BOOST_CHECK (boost::filesystem::exists(file));
+
+       BOOST_CHECK (equal(info1, writer, file, 0, EYES_LEFT));
+       BOOST_CHECK (equal(info2, writer, file, 5, EYES_RIGHT));
+
+       dcp::FrameInfo info3(12494, 99157123, "xxxxyyyyabc12356ffsfdsf456339522");
+       writer.write_frame_info (10, EYES_LEFT, info3);
+       BOOST_CHECK (boost::filesystem::exists(file));
+
+       BOOST_CHECK (equal(info1, writer, file, 0, EYES_LEFT));
+       BOOST_CHECK (equal(info2, writer, file, 5, EYES_RIGHT));
+       BOOST_CHECK (equal(info3, writer, file, 10, EYES_LEFT));
+
+       /* Overwrite one */
+
+       dcp::FrameInfo info4(55512494, 123599157123, "ABCDEFGyabc12356ffsfdsf4563395ZZ");
+       writer.write_frame_info (5, EYES_RIGHT, info4);
+       BOOST_CHECK (boost::filesystem::exists(file));
+
+       BOOST_CHECK (equal(info1, writer, file, 0, EYES_LEFT));
+       BOOST_CHECK (equal(info4, writer, file, 5, EYES_RIGHT));
+       BOOST_CHECK (equal(info3, writer, file, 10, EYES_LEFT));
+}

diff --git a/test/wscript b/test/wscript
index 06687cd178c3c4b28e5aa2aabd6105c9a714086c..1d2fafc02627c96a53a331d342c167fbaa4bec1c 100644 (file)
--- a/test/wscript
+++ b/test/wscript
@@ -96,6 +96,7 @@ def build(bld):
                  recover_test.cc
                  rect_test.cc
                  reels_test.cc
+                 reel_writer_test.cc
                  required_disk_space_test.cc
                  remake_id_test.cc
                  remake_with_subtitle_test.cc