From 48b82de5b6e8e07330a2f72dbddd8d9830fe047e Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 21 Oct 2019 17:25:04 +0200 Subject: [PATCH] Copy with progress updates when we might copy long files (#1574). --- src/lib/reel_writer.cc | 37 +++++++++++++++++++++++------- src/lib/reel_writer.h | 4 +++- src/lib/util.cc | 52 ++++++++++++++++++++++++++++++++++++++++++ src/lib/util.h | 3 ++- test/test.cc | 23 +++++++++++++++++++ test/test.h | 1 + test/util_test.cc | 26 ++++++++++++++++++++- 7 files changed, 135 insertions(+), 11 deletions(-) diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc index f6313773e..eeea48e42 100644 --- a/src/lib/reel_writer.cc +++ b/src/lib/reel_writer.cc @@ -54,6 +54,7 @@ using std::list; using std::string; using std::cout; +using std::exception; using std::map; using boost::shared_ptr; using boost::optional; @@ -75,6 +76,7 @@ ReelWriter::ReelWriter ( , _reel_index (reel_index) , _reel_count (reel_count) , _content_summary (content_summary) + , _job (job) { /* Create our picture asset in a subdirectory, named according to those film's parameters which affect the video output. We will hard-link @@ -100,9 +102,6 @@ ReelWriter::ReelWriter ( _film->internal_video_asset_dir() / _film->internal_video_asset_filename(_period) ); - if (job) { - job->sub (_("Checking existing image data")); - } _first_nonexistant_frame = check_existing_picture_asset (); _picture_asset_writer = _picture_asset->start_write ( @@ -180,16 +179,27 @@ ReelWriter::check_existing_picture_asset () DCPOMATIC_ASSERT (_picture_asset->file()); boost::filesystem::path asset = _picture_asset->file().get(); + shared_ptr job = _job.lock (); + /* If there is an existing asset, break any hard links to it as we are about to change its contents (if only by changing the IDs); see #1126. */ if (boost::filesystem::exists(asset) && boost::filesystem::hard_link_count(asset) > 1) { - boost::filesystem::copy_file (asset, asset.string() + ".tmp"); + if (job) { + job->sub (_("Copying old video file")); + copy_in_bits (asset, asset.string() + ".tmp", bind(&Job::set_progress, job.get(), _1, false)); + } else { + boost::filesystem::copy_file (asset, asset.string() + ".tmp"); + } boost::filesystem::remove (asset); boost::filesystem::rename (asset.string() + ".tmp", asset); } + if (job) { + job->sub (_("Checking existing image data")); + } + /* Try to open the existing asset */ FILE* asset_file = fopen_boost (asset, "rb"); if (!asset_file) { @@ -295,10 +305,21 @@ ReelWriter::finish () boost::filesystem::create_hard_link (video_from, video_to, ec); if (ec) { LOG_WARNING_NC ("Hard-link failed; copying instead"); - boost::filesystem::copy_file (video_from, video_to, ec); - if (ec) { - LOG_ERROR ("Failed to copy video file from %1 to %2 (%3)", video_from.string(), video_to.string(), ec.message ()); - throw FileError (ec.message(), video_from); + shared_ptr job = _job.lock (); + if (job) { + job->sub (_("Copying video file into DCP")); + try { + copy_in_bits (video_from, video_to, bind(&Job::set_progress, job.get(), _1, false)); + } catch (exception& e) { + LOG_ERROR ("Failed to copy video file from %1 to %2 (%3)", video_from.string(), video_to.string(), e.what()); + throw FileError (e.what(), video_from); + } + } else { + boost::filesystem::copy_file (video_from, video_to, ec); + if (ec) { + LOG_ERROR ("Failed to copy video file from %1 to %2 (%3)", video_from.string(), video_to.string(), ec.message ()); + throw FileError (ec.message(), video_from); + } } } diff --git a/src/lib/reel_writer.h b/src/lib/reel_writer.h index 8649ea37f..46f477616 100644 --- a/src/lib/reel_writer.h +++ b/src/lib/reel_writer.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2018 Carl Hetherington + Copyright (C) 2012-2019 Carl Hetherington This file is part of DCP-o-matic. @@ -25,6 +25,7 @@ #include "dcp_text_track.h" #include #include +#include namespace dcpomatic { class Font; @@ -116,6 +117,7 @@ private: /** number of reels in the DCP */ int _reel_count; boost::optional _content_summary; + boost::weak_ptr _job; boost::shared_ptr _picture_asset; boost::shared_ptr _picture_asset_writer; diff --git a/src/lib/util.cc b/src/lib/util.cc index cd2d5a368..234c22246 100644 --- a/src/lib/util.cc +++ b/src/lib/util.cc @@ -1015,6 +1015,58 @@ show_jobs_on_console (bool progress) return error; } +/** XXX: could use mmap? */ +void +copy_in_bits (boost::filesystem::path from, boost::filesystem::path to, boost::function progress) +{ + FILE* f = fopen_boost (from, "rb"); + if (!f) { + throw OpenFileError (from, errno, OpenFileError::READ); + } + FILE* t = fopen_boost (to, "wb"); + if (!t) { + fclose (f); + throw OpenFileError (to, errno, OpenFileError::WRITE); + } + + /* on the order of a second's worth of copying */ + boost::uintmax_t const chunk = 20 * 1024 * 1024; + + uint8_t* buffer = static_cast (malloc(chunk)); + if (!buffer) { + throw std::bad_alloc (); + } + + boost::uintmax_t const total = boost::filesystem::file_size (from); + boost::uintmax_t remaining = total; + + while (remaining) { + boost::uintmax_t this_time = min (chunk, remaining); + size_t N = fread (buffer, 1, chunk, f); + if (N < this_time) { + fclose (f); + fclose (t); + free (buffer); + throw ReadFileError (from, errno); + } + + N = fwrite (buffer, 1, this_time, t); + if (N < this_time) { + fclose (f); + fclose (t); + free (buffer); + throw WriteFileError (to, errno); + } + + progress (1 - float(remaining) / total); + remaining -= this_time; + } + + fclose (f); + fclose (t); + free (buffer); +} + #ifdef DCPOMATIC_VARIANT_SWAROOP /* Make up a key from the machine UUID */ diff --git a/src/lib/util.h b/src/lib/util.h index d90053cdc..c8dcb29d6 100644 --- a/src/lib/util.h +++ b/src/lib/util.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2016 Carl Hetherington + Copyright (C) 2012-2019 Carl Hetherington This file is part of DCP-o-matic. @@ -111,6 +111,7 @@ extern size_t utf8_strlen (std::string s); extern std::string day_of_week_to_string (boost::gregorian::greg_weekday d); extern void emit_subtitle_image (dcpomatic::ContentTimePeriod period, dcp::SubtitleImage sub, dcp::Size size, boost::shared_ptr decoder); extern bool show_jobs_on_console (bool progress); +extern void copy_in_bits (boost::filesystem::path from, boost::filesystem::path to, boost::function); #ifdef DCPOMATIC_VARIANT_SWAROOP extern boost::shared_ptr read_swaroop_chain (boost::filesystem::path path); extern void write_swaroop_chain (boost::shared_ptr chain, boost::filesystem::path output); diff --git a/test/test.cc b/test/test.cc index c87f4e70b..50770a687 100644 --- a/test/test.cc +++ b/test/test.cc @@ -495,3 +495,26 @@ subtitle_file (shared_ptr film) /* Remove warning */ return boost::filesystem::path("/"); } + +void +make_random_file (boost::filesystem::path path, size_t size) +{ + size_t const chunk = 128 * 1024; + uint8_t* buffer = static_cast (malloc(chunk)); + BOOST_REQUIRE (buffer); + FILE* r = fopen("/dev/urandom", "rb"); + BOOST_REQUIRE (r); + FILE* t = fopen_boost(path, "wb"); + BOOST_REQUIRE (t); + while (size) { + size_t this_time = min (size, chunk); + size_t N = fread (buffer, 1, this_time, r); + BOOST_REQUIRE (N == this_time); + N = fwrite (buffer, 1, this_time, t); + BOOST_REQUIRE (N == this_time); + size -= this_time; + } + fclose (t); + fclose (r); + free (buffer); +} diff --git a/test/test.h b/test/test.h index 4020dc772..86b13cde5 100644 --- a/test/test.h +++ b/test/test.h @@ -42,3 +42,4 @@ extern void write_image (boost::shared_ptr image, boost::filesystem boost::filesystem::path dcp_file (boost::shared_ptr film, std::string prefix); void check_one_frame (boost::filesystem::path dcp, int64_t index, boost::filesystem::path ref); extern boost::filesystem::path subtitle_file (boost::shared_ptr film); +extern void make_random_file (boost::filesystem::path path, size_t size); diff --git a/test/util_test.cc b/test/util_test.cc index 709bb0827..1c1091f28 100644 --- a/test/util_test.cc +++ b/test/util_test.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2016 Carl Hetherington + Copyright (C) 2012-2019 Carl Hetherington This file is part of DCP-o-matic. @@ -26,11 +26,14 @@ #include "lib/util.h" #include "lib/cross.h" #include "lib/exceptions.h" +#include "test.h" #include #include +#include using std::string; using std::vector; +using std::list; using boost::shared_ptr; using namespace dcpomatic; @@ -129,3 +132,24 @@ BOOST_AUTO_TEST_CASE (careful_string_filter_test) BOOST_CHECK_EQUAL ("hello_world", careful_string_filter("héllo_wörld")); BOOST_CHECK_EQUAL ("hello_world_a", careful_string_filter("héllo_wörld_à")); } + +static list progress_values; + +static void +progress (float p) +{ + progress_values.push_back (p); +} + +BOOST_AUTO_TEST_CASE (copy_in_bits_test) +{ + for (int i = 0; i < 32; ++i) { + make_random_file ("build/test/random.dat", rand() % (256 * 1024 * 1024)); + + progress_values.clear (); + copy_in_bits ("build/test/random.dat", "build/test/random.dat2", boost::bind(&progress, _1)); + BOOST_CHECK (!progress_values.empty()); + + check_file ("build/test/random.dat", "build/test/random.dat2"); + } +} -- 2.30.2