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 481d2e89dd668fd82b7e78c5651a0df1a2482e84..bf474e3ead2067ef5e6d1319b8a6ca5a2f27d827 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 fe87ababcad292335b52ed07d0328c19484420bb..766d3d8d5442f8cd2e2bb38580dcdafa8a853f68 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 fb34379106c72b8c5ae15756812e86e56dcdc9b7..f26d1d2cf224530582f709482840a2fb6840be39 100644 (file)
--- a/src/lib/ffmpeg.cc
+++ b/src/lib/ffmpeg.cc
@@ -128,7 +128,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 b7ccc07ce61fb6d05c9279fbca2e13db903420f2..355fe5c3bfac1e1982f2135aa992245e125de50e 100644 (file)
--- a/src/lib/reel_writer.cc
+++ b/src/lib/reel_writer.cc
@@ -62,6 +62,7 @@ using dcp::raw_convert;
 
 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
        )
@@ -97,7 +98,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 (
@@ -139,8 +142,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 ae64c3ac77ed9c73bd6f4f6939c9f5c6710548cf..741b0914cc9dc2f028844a71797399a164b0f215 100644 (file)
--- a/src/lib/reel_writer.h
+++ b/src/lib/reel_writer.h
@@ -30,6 +30,7 @@ class Film;
 class Job;
 class Font;
 class AudioBuffers;
+struct write_frame_info_test;
 
 namespace dcp {
        class MonoPictureAsset;
@@ -89,6 +90,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 340af1ea839ba8c96c75f39fcc7f671ea254255b..a17ac54f5ae3b2466a24941d6027b7af0c29d5d7 100644 (file)
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -471,7 +471,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]));
@@ -491,7 +491,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 d0f0825f13a3a5390fbe166fe37e461885114d2b..8666d90c871fc7cee962b90f5b7064942db9b2e1 100644 (file)
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -584,7 +584,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 743953650e8953e395409757a9bdcb8f8db5e49c..fecb55ea7ced2c0fcbc413f211da4ad7615f7de9 100644 (file)
--- a/src/wx/config_dialog.cc
+++ b/src/wx/config_dialog.cc
@@ -578,7 +578,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);
@@ -600,7 +600,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();
@@ -774,7 +774,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 ();
@@ -867,7 +867,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();
@@ -902,7 +902,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;
@@ -954,7 +954,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 5e7634ea37fb7745bdb44a91017a610a3b14488f..db773fb4692e32cd64a7535e108ec18176cf5d9a 100644 (file)
--- a/test/wscript
+++ b/test/wscript
@@ -94,6 +94,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