From 7a91ca1d866216e0086d46a430d5666c5525c620 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Mon, 8 Jun 2020 15:58:17 -0400 Subject: [PATCH 1/4] FFmpegWriter: Remove unreachable branch --- src/FFmpegWriter.cpp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index e6a7e5ef..3366ac5f 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -552,7 +552,7 @@ void FFmpegWriter::SetOption(StreamType stream, std::string name, std::string va } else if (strstr(info.vcodec.c_str(), "rav1e") != NULL) { // Set number of tiles to a fixed value - // TODO Let user choose number of tiles + // TODO Let user choose number of tiles av_opt_set_int(c->priv_data, "qp", std::min(std::stoi(value),255), 0); av_opt_set_int(c->priv_data, "speed", 7, 0); av_opt_set_int(c->priv_data, "tile-rows", 2, 0); // number of rows @@ -560,7 +560,7 @@ void FFmpegWriter::SetOption(StreamType stream, std::string name, std::string va } else if (strstr(info.vcodec.c_str(), "aom") != NULL) { // Set number of tiles to a fixed value - // TODO Let user choose number of tiles + // TODO Let user choose number of tiles // libaom doesn't have qp only crf av_opt_set_int(c->priv_data, "crf", std::min(std::stoi(value),63), 0); av_opt_set_int(c->priv_data, "tile-rows", 1, 0); // log2 of number of rows @@ -691,15 +691,8 @@ void FFmpegWriter::WriteFrame(std::shared_ptr frame) { // Write the frames once it reaches the correct cache size if ((int)spooled_video_frames.size() == cache_size || (int)spooled_audio_frames.size() == cache_size) { - // Is writer currently writing? - if (!is_writing) - // Write frames to video file - write_queued_frames(); - - else { - // Write frames to video file - write_queued_frames(); - } + // Write frames to video file + write_queued_frames(); } // Keep track of the last frame added From c57f8ede3b54776b399b6671f284f7946df757fc Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Mon, 8 Jun 2020 16:02:02 -0400 Subject: [PATCH 2/4] FFmpeg: Combine 2 constructors via a default arg, fix docs, const refs --- include/FFmpegReader.h | 15 +++++++-------- include/FFmpegWriter.h | 6 ++++-- src/FFmpegReader.cpp | 23 +---------------------- src/FFmpegWriter.cpp | 2 +- 4 files changed, 13 insertions(+), 33 deletions(-) diff --git a/include/FFmpegReader.h b/include/FFmpegReader.h index ec082965..35e11a42 100644 --- a/include/FFmpegReader.h +++ b/include/FFmpegReader.h @@ -233,14 +233,13 @@ namespace openshot { /// codecs have trouble seeking, and can introduce artifacts or blank images into the video. bool enable_seek; - /// Constructor for FFmpegReader. This automatically opens the media file and loads - /// frame 1, or it throws one of the following exceptions. - FFmpegReader(std::string path); - - /// Constructor for FFmpegReader. This only opens the media file to inspect its properties - /// if inspect_reader=true. When not inspecting the media file, it's much faster, and useful - /// when you are inflating the object using JSON after instantiating it. - FFmpegReader(std::string path, bool inspect_reader); + /// @brief Constructor for FFmpegReader. + /// + /// Sets (and possibly opens) the media file path, + /// or throws an exception. + /// @param path The filesystem location to load + /// @param inspect_reader if true (the default), automatically open the media file and loads frame 1. + FFmpegReader(const std::string& path, bool inspect_reader=true); /// Destructor virtual ~FFmpegReader(); diff --git a/include/FFmpegWriter.h b/include/FFmpegWriter.h index 37fa22dc..f2e28d00 100644 --- a/include/FFmpegWriter.h +++ b/include/FFmpegWriter.h @@ -251,9 +251,11 @@ namespace openshot { public: - /// @brief Constructor for FFmpegWriter. Throws one of the following exceptions. + /// @brief Constructor for FFmpegWriter. + /// Throws an exception on failure to open path. + /// /// @param path The file path of the video file you want to open and read - FFmpegWriter(std::string path); + FFmpegWriter(const std::string& path); /// Close the writer void Close(); diff --git a/src/FFmpegReader.cpp b/src/FFmpegReader.cpp index c8ce141f..12b9776a 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -83,7 +83,7 @@ int hw_de_on = 0; AVHWDeviceType hw_de_av_device_type_global = AV_HWDEVICE_TYPE_NONE; #endif -FFmpegReader::FFmpegReader(std::string path) +FFmpegReader::FFmpegReader(const std::string& path, bool inspect_reader) : last_frame(0), is_seeking(0), seeking_pts(0), seeking_frame(0), seek_count(0), audio_pts_offset(99999), video_pts_offset(99999), path(path), is_video_seek(true), check_interlace(false), check_fps(false), enable_seek(true), is_open(false), seek_audio_frame_found(0), seek_video_frame_found(0), @@ -91,27 +91,6 @@ FFmpegReader::FFmpegReader(std::string path) current_video_frame(0), has_missing_frames(false), num_packets_since_video_frame(0), num_checks_since_final(0), packet(NULL) { - // Initialize FFMpeg, and register all formats and codecs - AV_REGISTER_ALL - AVCODEC_REGISTER_ALL - - // Init cache - working_cache.SetMaxBytesFromInfo(OPEN_MP_NUM_PROCESSORS * info.fps.ToDouble() * 2, info.width, info.height, info.sample_rate, info.channels); - missing_frames.SetMaxBytesFromInfo(OPEN_MP_NUM_PROCESSORS * 2, info.width, info.height, info.sample_rate, info.channels); - final_cache.SetMaxBytesFromInfo(OPEN_MP_NUM_PROCESSORS * 2, info.width, info.height, info.sample_rate, info.channels); - - // Open and Close the reader, to populate its attributes (such as height, width, etc...) - Open(); - Close(); -} - -FFmpegReader::FFmpegReader(std::string path, bool inspect_reader) - : last_frame(0), is_seeking(0), seeking_pts(0), seeking_frame(0), seek_count(0), - audio_pts_offset(99999), video_pts_offset(99999), path(path), is_video_seek(true), check_interlace(false), - check_fps(false), enable_seek(true), is_open(false), seek_audio_frame_found(0), seek_video_frame_found(0), - prev_samples(0), prev_pts(0), pts_total(0), pts_counter(0), is_duration_known(false), largest_frame_processed(0), - current_video_frame(0), has_missing_frames(false), num_packets_since_video_frame(0), num_checks_since_final(0), - packet(NULL) { // Initialize FFMpeg, and register all formats and codecs AV_REGISTER_ALL diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 3366ac5f..eff2507e 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -83,7 +83,7 @@ static int set_hwframe_ctx(AVCodecContext *ctx, AVBufferRef *hw_device_ctx, int6 } #endif // HAVE_HW_ACCEL -FFmpegWriter::FFmpegWriter(std::string path) : +FFmpegWriter::FFmpegWriter(const std::string& path) : path(path), fmt(NULL), oc(NULL), audio_st(NULL), video_st(NULL), samples(NULL), audio_outbuf(NULL), audio_outbuf_size(0), audio_input_frame_size(0), audio_input_position(0), initial_audio_input_frame_size(0), img_convert_ctx(NULL), cache_size(8), num_of_rescalers(32), From 96d84311c68a3fee3e982ca51fd2951bf5bc4c2a Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Mon, 8 Jun 2020 16:07:04 -0400 Subject: [PATCH 3/4] OpenMP: Move off deprecated allow_nested boolean - Replacement openmp_set_max_active_levels() takes an int argument - In OpenMP 5.0 it can be set to openmp_get_supported_active_levels() - Below 5.0, we just set it to our processor count - Move configuration of OpenMP from ad-hoc locations to constructors for FFmpegWriter/Reader and Timeline --- include/OpenMPUtilities.h | 7 +++++++ src/FFmpegReader.cpp | 10 +++++----- src/FFmpegWriter.cpp | 11 ++++++----- src/Timeline.cpp | 18 +++++++++++++----- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/include/OpenMPUtilities.h b/include/OpenMPUtilities.h index 9810d636..5b5107c1 100644 --- a/include/OpenMPUtilities.h +++ b/include/OpenMPUtilities.h @@ -41,5 +41,12 @@ #define OPEN_MP_NUM_PROCESSORS (std::min(omp_get_num_procs(), std::max(2, openshot::Settings::Instance()->OMP_THREADS) )) #define FF_NUM_PROCESSORS (std::min(omp_get_num_procs(), std::max(2, openshot::Settings::Instance()->FF_THREADS) )) +// Set max-active-levels to the max supported, if possible +// (supported_active_levels is OpenMP 5.0 (November 2018) or later, only.) +#if (_OPENMP >= 201811) + #define OPEN_MP_MAX_ACTIVE openmp_get_supported_active_levels() +#else + #define OPEN_MP_MAX_ACTIVE OPEN_MP_NUM_PROCESSORS +#endif #endif diff --git a/src/FFmpegReader.cpp b/src/FFmpegReader.cpp index 12b9776a..fea4f60b 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -91,6 +91,11 @@ FFmpegReader::FFmpegReader(const std::string& path, bool inspect_reader) current_video_frame(0), has_missing_frames(false), num_packets_since_video_frame(0), num_checks_since_final(0), packet(NULL) { + // Configure OpenMP parallelism + // Default number of threads per section + omp_set_num_threads(OPEN_MP_NUM_PROCESSORS); + // Allow nested parallel sections as deeply as supported + omp_set_max_active_levels(OPEN_MP_MAX_ACTIVE); // Initialize FFMpeg, and register all formats and codecs AV_REGISTER_ALL @@ -877,11 +882,6 @@ std::shared_ptr FFmpegReader::ReadStream(int64_t requested_frame) { int minimum_packets = OPEN_MP_NUM_PROCESSORS; int max_packets = 4096; - // Set the number of threads in OpenMP - omp_set_num_threads(OPEN_MP_NUM_PROCESSORS); - // Allow nested OpenMP sections - omp_set_nested(true); - // Debug output ZmqLogger::Instance()->AppendDebugMethod("FFmpegReader::ReadStream", "requested_frame", requested_frame, "OPEN_MP_NUM_PROCESSORS", OPEN_MP_NUM_PROCESSORS); diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index eff2507e..39a3768f 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -95,6 +95,12 @@ FFmpegWriter::FFmpegWriter(const std::string& path) : info.has_audio = false; info.has_video = false; + // Configure OpenMP parallelism + // Default number of threads per block + omp_set_num_threads(OPEN_MP_NUM_PROCESSORS); + // Allow nested parallel sections as deeply as supported + omp_set_max_active_levels(OPEN_MP_MAX_ACTIVE); + // Initialize FFMpeg, and register all formats and codecs AV_REGISTER_ALL @@ -714,11 +720,6 @@ void FFmpegWriter::write_queued_frames() { spooled_video_frames.clear(); spooled_audio_frames.clear(); - // Set the number of threads in OpenMP - omp_set_num_threads(OPEN_MP_NUM_PROCESSORS); - // Allow nested OpenMP sections - omp_set_nested(true); - // Create blank exception bool has_error_encoding_video = false; diff --git a/src/Timeline.cpp b/src/Timeline.cpp index 124058ac..89f0992c 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -67,7 +67,13 @@ Timeline::Timeline(int width, int height, Fraction fps, int sample_rate, int cha info.acodec = "openshot::timeline"; info.vcodec = "openshot::timeline"; - // Init max image size + // Configure OpenMP parallelism + // Default number of threads per block + omp_set_num_threads(OPEN_MP_NUM_PROCESSORS); + // Allow nested parallel sections as deeply as supported + omp_set_max_active_levels(OPEN_MP_MAX_ACTIVE); + + // Init max image size SetMaxSize(info.width, info.height); // Init cache @@ -194,6 +200,12 @@ Timeline::Timeline(std::string projectPath, bool convert_absolute_paths) : info.has_video = true; info.has_audio = true; + // Configure OpenMP parallelism + // Default number of threads per section + omp_set_num_threads(OPEN_MP_NUM_PROCESSORS); + // Allow nested parallel sections as deeply as supported + omp_set_max_active_levels(OPEN_MP_MAX_ACTIVE); + // Init max image size SetMaxSize(info.width, info.height); @@ -892,10 +904,6 @@ std::shared_ptr Timeline::GetFrame(int64_t requested_frame) #pragma omp critical (T_GetFrame) nearby_clips = find_intersecting_clips(requested_frame, minimum_frames, true); - omp_set_num_threads(OPEN_MP_NUM_PROCESSORS); - // Allow nested OpenMP sections - omp_set_nested(true); - // Debug output ZmqLogger::Instance()->AppendDebugMethod("Timeline::GetFrame", "requested_frame", requested_frame, "minimum_frames", minimum_frames, "OPEN_MP_NUM_PROCESSORS", OPEN_MP_NUM_PROCESSORS); From 3f13ed87f0e18e636376be67847f9993670c0302 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Mon, 8 Jun 2020 16:07:55 -0400 Subject: [PATCH 4/4] Timeline: const-ref path arg in constructor, docs --- include/Timeline.h | 22 +++++++++++++--------- src/Timeline.cpp | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/include/Timeline.h b/include/Timeline.h index 932b04ac..32eac6be 100644 --- a/include/Timeline.h +++ b/include/Timeline.h @@ -202,19 +202,23 @@ namespace openshot { public: - /// @brief Default Constructor for the timeline (which sets the canvas width and height and FPS) - /// @param width The width of the timeline (and thus, the generated openshot::Frame objects) - /// @param height The height of the timeline (and thus, the generated openshot::Frame objects) - /// @param fps The frames rate of the timeline - /// @param sample_rate The sample rate of the timeline's audio - /// @param channels The number of audio channels of the timeline + /// @brief Default Constructor for the timeline (which configures the default frame properties) + /// @param width The image width of generated openshot::Frame objects + /// @param height The image height of generated openshot::Frame objects + /// @param fps The frame rate of the generated video + /// @param sample_rate The audio sample rate + /// @param channels The number of audio channels /// @param channel_layout The channel layout (i.e. mono, stereo, 3 point surround, etc...) Timeline(int width, int height, Fraction fps, int sample_rate, int channels, ChannelLayout channel_layout); - /// @brief Constructor for the timeline (which loads a JSON structure from a file path, and initializes a timeline) + /// @brief Project-file constructor for the timeline + /// + /// Loads a JSON structure from a file path, and + /// initializes the timeline described within. + /// /// @param projectPath The path of the UTF-8 *.osp project file (JSON contents). Contents will be loaded automatically. - /// @param convert_absolute_paths Should all paths be converted to absolute paths (based on the folder of the path provided) - Timeline(std::string projectPath, bool convert_absolute_paths); + /// @param convert_absolute_paths Should all paths be converted to absolute paths (relative to the location of projectPath) + Timeline(const std::string& projectPath, bool convert_absolute_paths); virtual ~Timeline(); diff --git a/src/Timeline.cpp b/src/Timeline.cpp index 89f0992c..bbbc6858 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -82,7 +82,7 @@ Timeline::Timeline(int width, int height, Fraction fps, int sample_rate, int cha } // Constructor for the timeline (which loads a JSON structure from a file path, and initializes a timeline) -Timeline::Timeline(std::string projectPath, bool convert_absolute_paths) : +Timeline::Timeline(const std::string& projectPath, bool convert_absolute_paths) : is_open(false), auto_map_clips(true), managed_cache(true), path(projectPath) { // Create CrashHandler and Attach (incase of errors)