From: Carl Hetherington Date: Sun, 29 Sep 2019 21:28:57 +0000 (+0200) Subject: Improve OpenFileError so that it doesn't say "opening for read" X-Git-Tag: v2.15.20 X-Git-Url: https://main.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=ab0e8cdcafdcb83096012380f674b8280474e851 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 c86f98da2..99d4c3979 100644 --- 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 f6b3bd902..ddd1e3603 100644 --- 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 adc5c224c..0668f9f37 100644 --- 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 fa6cb288d..9c91d1d87 100644 --- a/src/lib/ffmpeg_image_proxy.cc +++ b/src/lib/ffmpeg_image_proxy.cc @@ -147,7 +147,7 @@ FFmpegImageProxy::image (optional) 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 942eb435d..3e8a7b79c 100644 --- 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 69709fa0d..263148902 100644 --- 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 film, DCPTimePeriod period, shared_ptr job, int reel_index, int reel_count, optional 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 fff2e0b9e..b96bcfc68 100644 --- 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 fee4a3c26..2dedc32c1 100644 --- a/src/lib/util.cc +++ b/src/lib/util.cc @@ -473,7 +473,7 @@ digest_head_tail (vector 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 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 3fc01571e..5b24d7491 100644 --- 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 a6b299bcb..98a1a1a8e 100644 --- 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 index 000000000..db63ca8bf --- /dev/null +++ b/test/reel_writer_test.cc @@ -0,0 +1,88 @@ +/* + Copyright (C) 2019 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 . + +*/ + +/** @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 + +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 = new_test_film2 ("write_frame_info_test"); + dcpomatic::DCPTimePeriod const period (dcpomatic::DCPTime(0), dcpomatic::DCPTime(96000)); + ReelWriter writer (film, period, shared_ptr(), 0, 1, optional()); + + /* 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 06687cd17..1d2fafc02 100644 --- 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