From d23197c9b64b183826ee0aa1d7b304ff78f8bc95 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Wed, 8 May 2019 14:00:55 -0500 Subject: [PATCH 01/19] Updating hwaccel table to use emojis (instead of words) take 3 --- include/QtImageReader.h | 7 ++++--- src/QtImageReader.cpp | 11 +++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/include/QtImageReader.h b/include/QtImageReader.h index 6b260f15..e4d14f9b 100644 --- a/include/QtImageReader.h +++ b/include/QtImageReader.h @@ -65,9 +65,10 @@ namespace openshot { private: string path; - std::shared_ptr image; ///> Original image (full quality) - std::shared_ptr cached_image; ///> Scaled for performance - bool is_open; + std::shared_ptr image; ///> Original image (full quality) + std::shared_ptr cached_image; ///> Scaled for performance + bool is_open; ///> Is Reader opened + QSize max_size; ///> Current max_size as calculated with Clip properties public: diff --git a/src/QtImageReader.cpp b/src/QtImageReader.cpp index c500d221..a9682bd9 100644 --- a/src/QtImageReader.cpp +++ b/src/QtImageReader.cpp @@ -130,6 +130,10 @@ void QtImageReader::Open() info.display_ratio.num = size.num; info.display_ratio.den = size.den; + // Set current max size + max_size.setWidth(info.width); + max_size.setHeight(info.height); + // Mark as "open" is_open = true; } @@ -209,8 +213,7 @@ std::shared_ptr QtImageReader::GetFrame(int64_t requested_frame) } // Scale image smaller (or use a previous scaled image) - if (!cached_image || (cached_image && cached_image->width() != max_width || cached_image->height() != max_height)) { - + if (!cached_image || (cached_image && max_size.width() != max_width || max_size.height() != max_height)) { #if USE_RESVG == 1 // If defined and found in CMake, utilize the libresvg for parsing // SVG files and rasterizing them to QImages. @@ -239,6 +242,10 @@ std::shared_ptr QtImageReader::GetFrame(int64_t requested_frame) cached_image = std::shared_ptr(new QImage(image->scaled(max_width, max_height, Qt::KeepAspectRatio, Qt::SmoothTransformation))); cached_image = std::shared_ptr(new QImage(cached_image->convertToFormat(QImage::Format_RGBA8888))); #endif + + // Set max size (to later determine if max_size is changed) + max_size.setWidth(max_width); + max_size.setHeight(max_height); } // Create or get frame object From 833fcb8e8e69dc0cffa15b19424942165d190c19 Mon Sep 17 00:00:00 2001 From: Chris Kirmse Date: Wed, 8 May 2019 14:53:23 -0700 Subject: [PATCH 02/19] fix a number of memory leaks - some were with libav functions - same were due to non-virtual destructors --- include/CacheBase.h | 2 ++ include/CacheMemory.h | 2 +- include/Clip.h | 2 +- include/ClipBase.h | 1 + include/FFmpegReader.h | 2 +- include/FFmpegUtilities.h | 6 +++--- include/Frame.h | 2 +- include/FrameMapper.h | 2 +- include/QtImageReader.h | 2 ++ include/ReaderBase.h | 2 ++ include/Timeline.h | 2 ++ src/CacheBase.cpp | 5 ++++- src/ClipBase.cpp | 5 ++++- src/FFmpegReader.cpp | 15 +++++++++++++-- src/FFmpegWriter.cpp | 22 +++++++--------------- src/Frame.cpp | 2 +- src/FrameMapper.cpp | 3 +++ src/QtImageReader.cpp | 6 +++++- src/ReaderBase.cpp | 3 +++ src/Timeline.cpp | 7 ++++++- 20 files changed, 63 insertions(+), 30 deletions(-) diff --git a/include/CacheBase.h b/include/CacheBase.h index aaef5320..c764b2d9 100644 --- a/include/CacheBase.h +++ b/include/CacheBase.h @@ -60,6 +60,8 @@ namespace openshot { /// @param max_bytes The maximum bytes to allow in the cache. Once exceeded, the cache will purge the oldest frames. CacheBase(int64_t max_bytes); + virtual ~CacheBase(); + /// @brief Add a Frame to the cache /// @param frame The openshot::Frame object needing to be cached. virtual void Add(std::shared_ptr frame) = 0; diff --git a/include/CacheMemory.h b/include/CacheMemory.h index 2f3f018b..29187799 100644 --- a/include/CacheMemory.h +++ b/include/CacheMemory.h @@ -71,7 +71,7 @@ namespace openshot { CacheMemory(int64_t max_bytes); // Default destructor - ~CacheMemory(); + virtual ~CacheMemory(); /// @brief Add a Frame to the cache /// @param frame The openshot::Frame object needing to be cached. diff --git a/include/Clip.h b/include/Clip.h index 346629e4..b1869a90 100644 --- a/include/Clip.h +++ b/include/Clip.h @@ -160,7 +160,7 @@ namespace openshot { Clip(ReaderBase* new_reader); /// Destructor - ~Clip(); + virtual ~Clip(); /// @brief Add an effect to the clip /// @param effect Add an effect to the clip. An effect can modify the audio or video of an openshot::Frame. diff --git a/include/ClipBase.h b/include/ClipBase.h index 3dae8a53..1c5534fc 100644 --- a/include/ClipBase.h +++ b/include/ClipBase.h @@ -69,6 +69,7 @@ namespace openshot { /// Constructor for the base clip ClipBase() { }; + virtual ~ClipBase(); // Compare a clip using the Position() property bool operator< ( ClipBase& a) { return (Position() < a.Position()); } diff --git a/include/FFmpegReader.h b/include/FFmpegReader.h index abf1af57..7ef44c50 100644 --- a/include/FFmpegReader.h +++ b/include/FFmpegReader.h @@ -243,7 +243,7 @@ namespace openshot { FFmpegReader(string path, bool inspect_reader); /// Destructor - ~FFmpegReader(); + virtual ~FFmpegReader(); /// Close File void Close(); diff --git a/include/FFmpegUtilities.h b/include/FFmpegUtilities.h index 0d12ba72..7e3e0070 100644 --- a/include/FFmpegUtilities.h +++ b/include/FFmpegUtilities.h @@ -209,9 +209,9 @@ #define AV_FORMAT_NEW_STREAM(oc, st_codec, av_codec, av_st) av_st = avformat_new_stream(oc, NULL);\ if (!av_st) \ throw OutOfMemory("Could not allocate memory for the video stream.", path); \ - c = avcodec_alloc_context3(av_codec); \ - st_codec = c; \ - av_st->codecpar->codec_id = av_codec->id; + avcodec_get_context_defaults3(av_st->codec, av_codec); \ + c = av_st->codec; \ + st_codec = c; #define AV_COPY_PARAMS_FROM_CONTEXT(av_stream, av_codec) avcodec_parameters_from_context(av_stream->codecpar, av_codec); #elif LIBAVFORMAT_VERSION_MAJOR >= 55 #define AV_REGISTER_ALL av_register_all(); diff --git a/include/Frame.h b/include/Frame.h index 66d8ccfa..56a2a3ec 100644 --- a/include/Frame.h +++ b/include/Frame.h @@ -159,7 +159,7 @@ namespace openshot Frame& operator= (const Frame& other); /// Destructor - ~Frame(); + virtual ~Frame(); /// Add (or replace) pixel data to the frame (based on a solid color) void AddColor(int new_width, int new_height, string new_color); diff --git a/include/FrameMapper.h b/include/FrameMapper.h index 216fe73f..1901cb91 100644 --- a/include/FrameMapper.h +++ b/include/FrameMapper.h @@ -170,7 +170,7 @@ namespace openshot FrameMapper(ReaderBase *reader, Fraction target_fps, PulldownType target_pulldown, int target_sample_rate, int target_channels, ChannelLayout target_channel_layout); /// Destructor - ~FrameMapper(); + virtual ~FrameMapper(); /// Change frame rate or audio mapping details void ChangeMapping(Fraction target_fps, PulldownType pulldown, int target_sample_rate, int target_channels, ChannelLayout target_channel_layout); diff --git a/include/QtImageReader.h b/include/QtImageReader.h index e4d14f9b..d26c9b79 100644 --- a/include/QtImageReader.h +++ b/include/QtImageReader.h @@ -81,6 +81,8 @@ namespace openshot /// when you are inflating the object using JSON after instantiating it. QtImageReader(string path, bool inspect_reader); + virtual ~QtImageReader(); + /// Close File void Close(); diff --git a/include/ReaderBase.h b/include/ReaderBase.h index b0a1b3db..efbd5bc7 100644 --- a/include/ReaderBase.h +++ b/include/ReaderBase.h @@ -107,6 +107,8 @@ namespace openshot /// Constructor for the base reader, where many things are initialized. ReaderBase(); + virtual ~ReaderBase(); + /// Information about the current media file ReaderInfo info; diff --git a/include/Timeline.h b/include/Timeline.h index 312add2e..97923153 100644 --- a/include/Timeline.h +++ b/include/Timeline.h @@ -205,6 +205,8 @@ namespace openshot { /// @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); + virtual ~Timeline(); + /// @brief Add an openshot::Clip to the timeline /// @param clip Add an openshot::Clip to the timeline. A clip can contain any type of Reader. void AddClip(Clip* clip); diff --git a/src/CacheBase.cpp b/src/CacheBase.cpp index cffd995d..874674c0 100644 --- a/src/CacheBase.cpp +++ b/src/CacheBase.cpp @@ -42,6 +42,9 @@ CacheBase::CacheBase(int64_t max_bytes) : max_bytes(max_bytes) { cacheCriticalSection = new CriticalSection(); }; +CacheBase::~CacheBase() { +}; + // Set maximum bytes to a different amount based on a ReaderInfo struct void CacheBase::SetMaxBytesFromInfo(int64_t number_of_frames, int width, int height, int sample_rate, int channels) { @@ -69,4 +72,4 @@ void CacheBase::SetJsonValue(Json::Value root) { // Set data from Json (if key is found) if (!root["max_bytes"].isNull()) max_bytes = atoll(root["max_bytes"].asString().c_str()); -} \ No newline at end of file +} diff --git a/src/ClipBase.cpp b/src/ClipBase.cpp index 80cad87d..b2926244 100644 --- a/src/ClipBase.cpp +++ b/src/ClipBase.cpp @@ -29,6 +29,9 @@ using namespace openshot; +ClipBase::~ClipBase() { +} + // Generate Json::JsonValue for this object Json::Value ClipBase::JsonValue() { @@ -108,4 +111,4 @@ Json::Value ClipBase::add_property_choice_json(string name, int value, int selec // return JsonValue return new_choice; -} \ No newline at end of file +} diff --git a/src/FFmpegReader.cpp b/src/FFmpegReader.cpp index acd3b55f..0b67da4e 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -576,6 +576,12 @@ void FFmpegReader::Close() { ZmqLogger::Instance()->AppendDebugMethod("FFmpegReader::Close", "", -1, "", -1, "", -1, "", -1, "", -1, "", -1); + if (packet) { + // Remove previous packet before getting next one + RemoveAVPacket(packet); + packet = NULL; + } + // Close the codec if (info.has_video) { avcodec_flush_buffers(pCodecCtx); @@ -624,6 +630,8 @@ void FFmpegReader::Close() { seek_video_frame_found = 0; current_video_frame = 0; has_missing_frames = false; + + last_video_frame.reset(); } } @@ -926,7 +934,9 @@ std::shared_ptr FFmpegReader::ReadStream(int64_t requested_frame) { // down processing considerably, but might be more stable on some systems. #pragma omp taskwait } - } + } else { + RemoveAVFrame(pFrame); + } } // Audio packet @@ -1063,7 +1073,7 @@ bool FFmpegReader::GetAVFrame() { { next_frame2 = next_frame; } - pFrame = new AVFrame(); + pFrame = AV_ALLOCATE_FRAME(); while (ret >= 0) { ret = avcodec_receive_frame(pCodecCtx, next_frame2); if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) { @@ -1206,6 +1216,7 @@ void FFmpegReader::ProcessVideoPacket(int64_t requested_frame) { int width = info.width; int64_t video_length = info.video_length; AVFrame *my_frame = pFrame; + pFrame = NULL; // Add video frame to list of processing video frames const GenericScopedLock lock(processingCriticalSection); diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index 072ac6d7..ddcd4e16 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -884,9 +884,8 @@ void FFmpegWriter::flush_encoders() { } // Close the video codec -void FFmpegWriter::close_video(AVFormatContext *oc, AVStream *st) { - AV_FREE_CONTEXT(video_codec); - video_codec = NULL; +void FFmpegWriter::close_video(AVFormatContext *oc, AVStream *st) +{ #if IS_FFMPEG_3_2 // #if defined(__linux__) if (hw_en_on && hw_en_supported) { @@ -900,10 +899,8 @@ void FFmpegWriter::close_video(AVFormatContext *oc, AVStream *st) { } // Close the audio codec -void FFmpegWriter::close_audio(AVFormatContext *oc, AVStream *st) { - AV_FREE_CONTEXT(audio_codec); - audio_codec = NULL; - +void FFmpegWriter::close_audio(AVFormatContext *oc, AVStream *st) +{ // Clear buffers delete[] samples; delete[] audio_outbuf; @@ -942,12 +939,6 @@ void FFmpegWriter::Close() { if (image_rescalers.size() > 0) RemoveScalers(); - // Free the streams - for (int i = 0; i < oc->nb_streams; i++) { - av_freep(AV_GET_CODEC_ATTRIBUTES(&oc->streams[i], &oc->streams[i])); - av_freep(&oc->streams[i]); - } - if (!(fmt->flags & AVFMT_NOFILE)) { /* close the output file */ avio_close(oc->pb); @@ -957,8 +948,9 @@ void FFmpegWriter::Close() { write_video_count = 0; write_audio_count = 0; - // Free the context - av_freep(&oc); + // Free the context which frees the streams too + avformat_free_context(oc); + oc = NULL; // Close writer is_open = false; diff --git a/src/Frame.cpp b/src/Frame.cpp index 24b653a9..6ef44d67 100644 --- a/src/Frame.cpp +++ b/src/Frame.cpp @@ -120,7 +120,7 @@ void Frame::DeepCopy(const Frame& other) wave_image = std::shared_ptr(new QImage(*(other.wave_image))); } -// Descructor +// Destructor Frame::~Frame() { // Clear all pointers image.reset(); diff --git a/src/FrameMapper.cpp b/src/FrameMapper.cpp index 73b7bb22..cf6955f2 100644 --- a/src/FrameMapper.cpp +++ b/src/FrameMapper.cpp @@ -61,6 +61,9 @@ FrameMapper::~FrameMapper() { if (is_open) // Auto Close if not already Close(); + + delete reader; + reader = NULL; } /// Get the current reader diff --git a/src/QtImageReader.cpp b/src/QtImageReader.cpp index a9682bd9..502ddb9a 100644 --- a/src/QtImageReader.cpp +++ b/src/QtImageReader.cpp @@ -57,6 +57,10 @@ QtImageReader::QtImageReader(string path, bool inspect_reader) : path(path), is_ } } +QtImageReader::~QtImageReader() +{ +} + // Open image file void QtImageReader::Open() { @@ -147,7 +151,7 @@ void QtImageReader::Close() { // Mark as "closed" is_open = false; - + // Delete the image image.reset(); diff --git a/src/ReaderBase.cpp b/src/ReaderBase.cpp index f2607cfd..3b1bb76f 100644 --- a/src/ReaderBase.cpp +++ b/src/ReaderBase.cpp @@ -63,6 +63,9 @@ ReaderBase::ReaderBase() parent = NULL; } +ReaderBase::~ReaderBase() { +} + // Display file information void ReaderBase::DisplayInfo() { cout << fixed << setprecision(2) << boolalpha; diff --git a/src/Timeline.cpp b/src/Timeline.cpp index b229a3de..e40de562 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -70,6 +70,11 @@ Timeline::Timeline(int width, int height, Fraction fps, int sample_rate, int cha final_cache->SetMaxBytesFromInfo(OPEN_MP_NUM_PROCESSORS * 2, info.width, info.height, info.sample_rate, info.channels); } +Timeline::~Timeline() { + delete final_cache; + final_cache = NULL; +} + // Add an openshot::Clip to the timeline void Timeline::AddClip(Clip* clip) { @@ -1481,4 +1486,4 @@ void Timeline::SetMaxSize(int width, int height) { // Set max size Settings::Instance()->MAX_WIDTH = display_ratio_size.width(); Settings::Instance()->MAX_HEIGHT = display_ratio_size.height(); -} \ No newline at end of file +} From d5a29500a52ccef98bf9cc00e0c20f1d4750fb53 Mon Sep 17 00:00:00 2001 From: Chris Kirmse Date: Thu, 9 May 2019 10:51:40 -0700 Subject: [PATCH 03/19] change freeing of frame_mappers allocated in Timeline - each class is now responsible to free whatever it allocates - all tests passed on my machine with ffmpeg 3.2 - Clip is now more careful about freeing a reader if it allocated it as well --- include/Clip.h | 5 ++++- include/DummyReader.h | 2 ++ include/Timeline.h | 2 ++ src/Clip.cpp | 19 ++++++++----------- src/DummyReader.cpp | 3 +++ src/FrameMapper.cpp | 1 - src/Timeline.cpp | 22 +++++++++++++++++++--- 7 files changed, 38 insertions(+), 16 deletions(-) diff --git a/include/Clip.h b/include/Clip.h index b1869a90..58eacab7 100644 --- a/include/Clip.h +++ b/include/Clip.h @@ -112,7 +112,10 @@ namespace openshot { // File Reader object ReaderBase* reader; - bool manage_reader; + + /// If we allocated a reader, we store it here to free it later + /// (reader member variable itself may have been replaced) + ReaderBase* allocated_reader; /// Adjust frame number minimum value int64_t adjust_frame_number_minimum(int64_t frame_number); diff --git a/include/DummyReader.h b/include/DummyReader.h index 559215de..e9bce1b5 100644 --- a/include/DummyReader.h +++ b/include/DummyReader.h @@ -64,6 +64,8 @@ namespace openshot /// Constructor for DummyReader. DummyReader(Fraction fps, int width, int height, int sample_rate, int channels, float duration); + virtual ~DummyReader(); + /// Close File void Close(); diff --git a/include/Timeline.h b/include/Timeline.h index 97923153..eecafec1 100644 --- a/include/Timeline.h +++ b/include/Timeline.h @@ -30,6 +30,7 @@ #include #include +#include #include #include #include "CacheBase.h" @@ -152,6 +153,7 @@ namespace openshot { map open_clips; /// effects; /// allocated_frame_mappers; /// all the frame mappers we allocated and must free /// Process a new layer of video or audio void add_layer(std::shared_ptr new_frame, Clip* source_clip, int64_t clip_frame_number, int64_t timeline_frame_number, bool is_top_clip, float max_volume); diff --git a/src/Clip.cpp b/src/Clip.cpp index 207494e3..2099705d 100644 --- a/src/Clip.cpp +++ b/src/Clip.cpp @@ -101,9 +101,6 @@ void Clip::init_settings() // Init audio and video overrides has_audio = Keyframe(-1.0); has_video = Keyframe(-1.0); - - // Default pointers - manage_reader = false; } // Init reader's rotation (if any) @@ -131,14 +128,14 @@ void Clip::init_reader_rotation() { } // Default Constructor for a clip -Clip::Clip() : reader(NULL), resampler(NULL), audio_cache(NULL) +Clip::Clip() : resampler(NULL), audio_cache(NULL), reader(NULL), allocated_reader(NULL) { // Init all default settings init_settings(); } // Constructor with reader -Clip::Clip(ReaderBase* new_reader) : reader(new_reader), resampler(NULL), audio_cache(NULL) +Clip::Clip(ReaderBase* new_reader) : resampler(NULL), audio_cache(NULL), reader(new_reader), allocated_reader(NULL) { // Init all default settings init_settings(); @@ -152,7 +149,7 @@ Clip::Clip(ReaderBase* new_reader) : reader(new_reader), resampler(NULL), audio_ } // Constructor with filepath -Clip::Clip(string path) : reader(NULL), resampler(NULL), audio_cache(NULL) +Clip::Clip(string path) : resampler(NULL), audio_cache(NULL), reader(NULL), allocated_reader(NULL) { // Init all default settings init_settings(); @@ -194,7 +191,7 @@ Clip::Clip(string path) : reader(NULL), resampler(NULL), audio_cache(NULL) // Update duration if (reader) { End(reader->info.duration); - manage_reader = true; + allocated_reader = reader; init_reader_rotation(); } } @@ -203,9 +200,9 @@ Clip::Clip(string path) : reader(NULL), resampler(NULL), audio_cache(NULL) Clip::~Clip() { // Delete the reader if clip created it - if (manage_reader && reader) { - delete reader; - reader = NULL; + if (allocated_reader) { + delete allocated_reader; + allocated_reader = NULL; } // Close the resampler @@ -968,7 +965,7 @@ void Clip::SetJsonValue(Json::Value root) { // mark as managed reader and set parent if (reader) { reader->SetClip(this); - manage_reader = true; + allocated_reader = reader; } // Re-Open reader (if needed) diff --git a/src/DummyReader.cpp b/src/DummyReader.cpp index 8fe039ab..dc77db5b 100644 --- a/src/DummyReader.cpp +++ b/src/DummyReader.cpp @@ -71,6 +71,9 @@ DummyReader::DummyReader(Fraction fps, int width, int height, int sample_rate, i Close(); } +DummyReader::~DummyReader() { +} + // Open image file void DummyReader::Open() { diff --git a/src/FrameMapper.cpp b/src/FrameMapper.cpp index cf6955f2..ca2038a9 100644 --- a/src/FrameMapper.cpp +++ b/src/FrameMapper.cpp @@ -62,7 +62,6 @@ FrameMapper::~FrameMapper() { // Auto Close if not already Close(); - delete reader; reader = NULL; } diff --git a/src/Timeline.cpp b/src/Timeline.cpp index e40de562..8692d17f 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -71,8 +71,12 @@ Timeline::Timeline(int width, int height, Fraction fps, int sample_rate, int cha } Timeline::~Timeline() { - delete final_cache; - final_cache = NULL; + if (is_open) + // Auto Close if not already + Close(); + + delete final_cache; + final_cache = NULL; } // Add an openshot::Clip to the timeline @@ -128,7 +132,9 @@ void Timeline::apply_mapper_to_clip(Clip* clip) } else { // Create a new FrameMapper to wrap the current reader - clip_reader = (ReaderBase*) new FrameMapper(clip->Reader(), info.fps, PULLDOWN_NONE, info.sample_rate, info.channels, info.channel_layout); + FrameMapper* mapper = new FrameMapper(clip->Reader(), info.fps, PULLDOWN_NONE, info.sample_rate, info.channels, info.channel_layout); + allocated_frame_mappers.insert(mapper); + clip_reader = (ReaderBase*) mapper; } // Update the mapping @@ -642,6 +648,16 @@ void Timeline::Close() update_open_clips(clip, false); } + // Free all allocated frame mappers + set::iterator frame_mapper_itr; + for (frame_mapper_itr=allocated_frame_mappers.begin(); frame_mapper_itr != allocated_frame_mappers.end(); ++frame_mapper_itr) + { + // Get frame mapper object from the iterator + FrameMapper *frame_mapper = (*frame_mapper_itr); + delete frame_mapper; + } + allocated_frame_mappers.clear(); + // Mark timeline as closed is_open = false; From 8ea0af59c60e77ce0ff260a134876ce511fa4c0c Mon Sep 17 00:00:00 2001 From: Chris Kirmse Date: Fri, 10 May 2019 11:39:26 -0700 Subject: [PATCH 04/19] fix allocations to be done the same for ffmpeg < 3.2 - fixes freeing an invalid pointer on old ffmpeg - now all tests pass --- src/FFmpegReader.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/FFmpegReader.cpp b/src/FFmpegReader.cpp index 0b67da4e..38d59f83 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -1120,11 +1120,13 @@ bool FFmpegReader::GetAVFrame() { #else avcodec_decode_video2(pCodecCtx, next_frame, &frameFinished, packet); + // always allocate pFrame (because we do that in the ffmpeg >= 3.2 as well); it will always be freed later + pFrame = AV_ALLOCATE_FRAME(); + // is frame finished if (frameFinished) { // AVFrames are clobbered on the each call to avcodec_decode_video, so we // must make a copy of the image data before this method is called again. - pFrame = AV_ALLOCATE_FRAME(); avpicture_alloc((AVPicture *) pFrame, pCodecCtx->pix_fmt, info.width, info.height); av_picture_copy((AVPicture *) pFrame, (AVPicture *) next_frame, pCodecCtx->pix_fmt, info.width, info.height); From bd21d1a751b3d1375cae37747e6d193e2e38d17c Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Mon, 13 May 2019 16:18:15 -0500 Subject: [PATCH 05/19] Fixing crash on Timeline::Close due to deleted FrameMappers --- src/Timeline.cpp | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/Timeline.cpp b/src/Timeline.cpp index 8692d17f..fd79364a 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -75,8 +75,20 @@ Timeline::~Timeline() { // Auto Close if not already Close(); - delete final_cache; - final_cache = NULL; + // Free all allocated frame mappers + set::iterator frame_mapper_itr; + for (frame_mapper_itr = allocated_frame_mappers.begin(); frame_mapper_itr != allocated_frame_mappers.end(); ++frame_mapper_itr) { + // Get frame mapper object from the iterator + FrameMapper *frame_mapper = (*frame_mapper_itr); + delete frame_mapper; + } + allocated_frame_mappers.clear(); + + // Remove cache + if (final_cache) { + delete final_cache; + final_cache = NULL; + } } // Add an openshot::Clip to the timeline @@ -648,16 +660,6 @@ void Timeline::Close() update_open_clips(clip, false); } - // Free all allocated frame mappers - set::iterator frame_mapper_itr; - for (frame_mapper_itr=allocated_frame_mappers.begin(); frame_mapper_itr != allocated_frame_mappers.end(); ++frame_mapper_itr) - { - // Get frame mapper object from the iterator - FrameMapper *frame_mapper = (*frame_mapper_itr); - delete frame_mapper; - } - allocated_frame_mappers.clear(); - // Mark timeline as closed is_open = false; From 968e472c73bccac9179a139fa7584a05acf8a95b Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Mon, 13 May 2019 17:11:40 -0500 Subject: [PATCH 06/19] Tweak how Timeline manages the cache object (sometimes itself, and sometimes by the user if they call SetCache) --- include/Timeline.h | 6 ++++-- src/Timeline.cpp | 13 ++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/include/Timeline.h b/include/Timeline.h index eecafec1..11911d40 100644 --- a/include/Timeline.h +++ b/include/Timeline.h @@ -153,7 +153,8 @@ namespace openshot { map open_clips; /// effects; /// allocated_frame_mappers; /// all the frame mappers we allocated and must free + set allocated_frame_mappers; ///< all the frame mappers we allocated and must free + bool managed_cache; ///< Does this timeline instance manage the cache object /// Process a new layer of video or audio void add_layer(std::shared_ptr new_frame, Clip* source_clip, int64_t clip_frame_number, int64_t timeline_frame_number, bool is_top_clip, float max_volume); @@ -241,7 +242,8 @@ namespace openshot { /// Get the cache object used by this reader CacheBase* GetCache() { return final_cache; }; - /// Get the cache object used by this reader + /// Set the cache object used by this reader. You must now manage the lifecycle + /// of this cache object though (Timeline will not delete it for you). void SetCache(CacheBase* new_cache); /// Get an openshot::Frame object for a specific frame number of this timeline. diff --git a/src/Timeline.cpp b/src/Timeline.cpp index fd79364a..91194399 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -31,7 +31,7 @@ using namespace openshot; // Default Constructor for the timeline (which sets the canvas width and height) Timeline::Timeline(int width, int height, Fraction fps, int sample_rate, int channels, ChannelLayout channel_layout) : - is_open(false), auto_map_clips(true) + is_open(false), auto_map_clips(true), managed_cache(true) { // Create CrashHandler and Attach (incase of errors) CrashHandler::Instance(); @@ -84,8 +84,8 @@ Timeline::~Timeline() { } allocated_frame_mappers.clear(); - // Remove cache - if (final_cache) { + // Destroy previous cache (if managed by timeline) + if (managed_cache && final_cache) { delete final_cache; final_cache = NULL; } @@ -923,6 +923,13 @@ vector Timeline::find_intersecting_clips(int64_t requested_frame, int num // Get the cache object used by this reader void Timeline::SetCache(CacheBase* new_cache) { + // Destroy previous cache (if managed by timeline) + if (managed_cache && final_cache) { + delete final_cache; + final_cache = NULL; + managed_cache = false; + } + // Set new cache final_cache = new_cache; } From 6335d6f06cfbfa796454eeabd19ac00547432579 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Mon, 13 May 2019 22:48:21 -0500 Subject: [PATCH 07/19] Adding debugging messaging to unit test which is failing on Travis CI --- tests/Timeline_Tests.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/Timeline_Tests.cpp b/tests/Timeline_Tests.cpp index 8c81579c..a59907b3 100644 --- a/tests/Timeline_Tests.cpp +++ b/tests/Timeline_Tests.cpp @@ -185,7 +185,9 @@ TEST(Timeline_Check_Two_Track_Video) TEST(Timeline_Clip_Order) { // Create a timeline + cout << "A" << endl; Timeline t(640, 480, Fraction(30, 1), 44100, 2, LAYOUT_STEREO); + cout << "B" << endl; // Add some clips out of order stringstream path_top; @@ -193,40 +195,51 @@ TEST(Timeline_Clip_Order) Clip clip_top(path_top.str()); clip_top.Layer(2); t.AddClip(&clip_top); + cout << "C" << endl; stringstream path_middle; path_middle << TEST_MEDIA_PATH << "front.png"; Clip clip_middle(path_middle.str()); clip_middle.Layer(0); t.AddClip(&clip_middle); + cout << "D" << endl; stringstream path_bottom; path_bottom << TEST_MEDIA_PATH << "back.png"; Clip clip_bottom(path_bottom.str()); clip_bottom.Layer(1); t.AddClip(&clip_bottom); + cout << "E" << endl; // Open Timeline t.Open(); + cout << "F" << endl; // Loop through Clips and check order (they should have been sorted into the correct order) // Bottom layer to top layer, then by position. list::iterator clip_itr; list clips = t.Clips(); + cout << "G" << endl; int counter = 0; for (clip_itr=clips.begin(); clip_itr != clips.end(); ++clip_itr) { + cout << "H" << endl; // Get clip object from the iterator Clip *clip = (*clip_itr); + cout << "I" << endl; + switch (counter) { case 0: + cout << "J" << endl; CHECK_EQUAL(0, clip->Layer()); break; case 1: + cout << "K" << endl; CHECK_EQUAL(1, clip->Layer()); break; case 2: + cout << "L" << endl; CHECK_EQUAL(2, clip->Layer()); break; } @@ -235,6 +248,8 @@ TEST(Timeline_Clip_Order) counter++; } + cout << "M" << endl; + // Add another clip stringstream path_middle1; path_middle1 << TEST_MEDIA_PATH << "interlaced.png"; @@ -243,28 +258,35 @@ TEST(Timeline_Clip_Order) clip_middle1.Position(0.5); t.AddClip(&clip_middle1); + cout << "N" << endl; // Loop through clips again, and re-check order counter = 0; clips = t.Clips(); + cout << "O" << endl; for (clip_itr=clips.begin(); clip_itr != clips.end(); ++clip_itr) { // Get clip object from the iterator Clip *clip = (*clip_itr); + cout << "P" << endl; switch (counter) { case 0: + cout << "Q" << endl; CHECK_EQUAL(0, clip->Layer()); break; case 1: + cout << "R" << endl; CHECK_EQUAL(1, clip->Layer()); CHECK_CLOSE(0.0, clip->Position(), 0.0001); break; case 2: + cout << "S" << endl; CHECK_EQUAL(1, clip->Layer()); CHECK_CLOSE(0.5, clip->Position(), 0.0001); break; case 3: + cout << "T" << endl; CHECK_EQUAL(2, clip->Layer()); break; } @@ -273,8 +295,12 @@ TEST(Timeline_Clip_Order) counter++; } + cout << "U" << endl; + // Close reader t.Close(); + + cout << "V" << endl; } From 9ffd6a6f754ac03200aec40b3a20820116f7f72d Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Mon, 13 May 2019 23:55:03 -0500 Subject: [PATCH 08/19] Fixing crash when destructing Timeline/Clips/FrameMapper --- include/FrameMapper.h | 3 +++ src/Timeline.cpp | 2 ++ tests/Timeline_Tests.cpp | 27 --------------------------- 3 files changed, 5 insertions(+), 27 deletions(-) diff --git a/include/FrameMapper.h b/include/FrameMapper.h index 1901cb91..d046af25 100644 --- a/include/FrameMapper.h +++ b/include/FrameMapper.h @@ -213,6 +213,9 @@ namespace openshot /// Get the current reader ReaderBase* Reader(); + /// Set the current reader + void Reader(ReaderBase *new_reader) { reader = new_reader; } + /// Resample audio and map channels (if needed) void ResampleMappedAudio(std::shared_ptr frame, int64_t original_frame_number); diff --git a/src/Timeline.cpp b/src/Timeline.cpp index 91194399..2ca24e84 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -80,6 +80,8 @@ Timeline::~Timeline() { for (frame_mapper_itr = allocated_frame_mappers.begin(); frame_mapper_itr != allocated_frame_mappers.end(); ++frame_mapper_itr) { // Get frame mapper object from the iterator FrameMapper *frame_mapper = (*frame_mapper_itr); + frame_mapper->Reader(NULL); + frame_mapper->Close(); delete frame_mapper; } allocated_frame_mappers.clear(); diff --git a/tests/Timeline_Tests.cpp b/tests/Timeline_Tests.cpp index a59907b3..22f33894 100644 --- a/tests/Timeline_Tests.cpp +++ b/tests/Timeline_Tests.cpp @@ -185,9 +185,7 @@ TEST(Timeline_Check_Two_Track_Video) TEST(Timeline_Clip_Order) { // Create a timeline - cout << "A" << endl; Timeline t(640, 480, Fraction(30, 1), 44100, 2, LAYOUT_STEREO); - cout << "B" << endl; // Add some clips out of order stringstream path_top; @@ -195,51 +193,40 @@ TEST(Timeline_Clip_Order) Clip clip_top(path_top.str()); clip_top.Layer(2); t.AddClip(&clip_top); - cout << "C" << endl; stringstream path_middle; path_middle << TEST_MEDIA_PATH << "front.png"; Clip clip_middle(path_middle.str()); clip_middle.Layer(0); t.AddClip(&clip_middle); - cout << "D" << endl; stringstream path_bottom; path_bottom << TEST_MEDIA_PATH << "back.png"; Clip clip_bottom(path_bottom.str()); clip_bottom.Layer(1); t.AddClip(&clip_bottom); - cout << "E" << endl; // Open Timeline t.Open(); - cout << "F" << endl; // Loop through Clips and check order (they should have been sorted into the correct order) // Bottom layer to top layer, then by position. list::iterator clip_itr; list clips = t.Clips(); - cout << "G" << endl; int counter = 0; for (clip_itr=clips.begin(); clip_itr != clips.end(); ++clip_itr) { - cout << "H" << endl; // Get clip object from the iterator Clip *clip = (*clip_itr); - cout << "I" << endl; - switch (counter) { case 0: - cout << "J" << endl; CHECK_EQUAL(0, clip->Layer()); break; case 1: - cout << "K" << endl; CHECK_EQUAL(1, clip->Layer()); break; case 2: - cout << "L" << endl; CHECK_EQUAL(2, clip->Layer()); break; } @@ -248,8 +235,6 @@ TEST(Timeline_Clip_Order) counter++; } - cout << "M" << endl; - // Add another clip stringstream path_middle1; path_middle1 << TEST_MEDIA_PATH << "interlaced.png"; @@ -258,35 +243,27 @@ TEST(Timeline_Clip_Order) clip_middle1.Position(0.5); t.AddClip(&clip_middle1); - cout << "N" << endl; - // Loop through clips again, and re-check order counter = 0; clips = t.Clips(); - cout << "O" << endl; for (clip_itr=clips.begin(); clip_itr != clips.end(); ++clip_itr) { // Get clip object from the iterator Clip *clip = (*clip_itr); - cout << "P" << endl; switch (counter) { case 0: - cout << "Q" << endl; CHECK_EQUAL(0, clip->Layer()); break; case 1: - cout << "R" << endl; CHECK_EQUAL(1, clip->Layer()); CHECK_CLOSE(0.0, clip->Position(), 0.0001); break; case 2: - cout << "S" << endl; CHECK_EQUAL(1, clip->Layer()); CHECK_CLOSE(0.5, clip->Position(), 0.0001); break; case 3: - cout << "T" << endl; CHECK_EQUAL(2, clip->Layer()); break; } @@ -295,12 +272,8 @@ TEST(Timeline_Clip_Order) counter++; } - cout << "U" << endl; - // Close reader t.Close(); - - cout << "V" << endl; } From 4a3985e209902f51798758766d1860956343b028 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Tue, 14 May 2019 00:20:32 -0500 Subject: [PATCH 09/19] Updating comment --- src/Timeline.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Timeline.cpp b/src/Timeline.cpp index 2ca24e84..37d3f71c 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -923,7 +923,7 @@ vector Timeline::find_intersecting_clips(int64_t requested_frame, int num return matching_clips; } -// Get the cache object used by this reader +// Set the cache object used by this reader void Timeline::SetCache(CacheBase* new_cache) { // Destroy previous cache (if managed by timeline) if (managed_cache && final_cache) { From fab70dde1ed88c5643672146e45808b43d9fef2a Mon Sep 17 00:00:00 2001 From: Chad Walker Date: Wed, 15 May 2019 10:27:48 -0500 Subject: [PATCH 10/19] plug another small leak --- src/FFmpegReader.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/FFmpegReader.cpp b/src/FFmpegReader.cpp index 38d59f83..e8f135ce 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -1038,6 +1038,8 @@ int FFmpegReader::GetNextPacket() { // Update current packet pointer packet = next_packet; } + else + delete next_packet; } // Return if packet was found (or error number) return found_packet; From 25e51d815efa3f30b337fd79bdd831ade6a88033 Mon Sep 17 00:00:00 2001 From: Chris Kirmse Date: Thu, 30 May 2019 09:40:18 -0700 Subject: [PATCH 11/19] free cache in FrameMapper::Close() - this hugely reduces the memory used by rendering a timeline with a lot of clips - could be related to issue #239 --- src/FrameMapper.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/FrameMapper.cpp b/src/FrameMapper.cpp index ca2038a9..113171a2 100644 --- a/src/FrameMapper.cpp +++ b/src/FrameMapper.cpp @@ -651,6 +651,16 @@ void FrameMapper::Close() // Close internal reader reader->Close(); + // Clear the fields & frames lists + fields.clear(); + frames.clear(); + + // Mark as dirty + is_dirty = true; + + // Clear cache + final_cache.Clear(); + // Deallocate resample buffer if (avr) { SWR_CLOSE(avr); From 13e74b147abe530e0373c836bcadaa4cf58fad84 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Fri, 31 May 2019 19:02:28 -0500 Subject: [PATCH 12/19] Adding new CheckPixel method to validate a specific pixel color --- include/Frame.h | 3 +++ src/Frame.cpp | 22 ++++++++++++++++++++++ tests/FFmpegReader_Tests.cpp | 8 ++++++++ 3 files changed, 33 insertions(+) diff --git a/include/Frame.h b/include/Frame.h index 56a2a3ec..72822188 100644 --- a/include/Frame.h +++ b/include/Frame.h @@ -249,6 +249,9 @@ namespace openshot /// Get pixel data (for only a single scan-line) const unsigned char* GetPixels(int row); + /// Check a specific pixel color value (returns True/False) + bool CheckPixel(int row, int col, int red, int green, int blue, int alpha); + /// Get height of image int GetHeight(); diff --git a/src/Frame.cpp b/src/Frame.cpp index 6ef44d67..27bf5f71 100644 --- a/src/Frame.cpp +++ b/src/Frame.cpp @@ -480,6 +480,28 @@ const unsigned char* Frame::GetPixels(int row) return image->scanLine(row); } +// Check a specific pixel color value (returns True/False) +bool Frame::CheckPixel(int row, int col, int red, int green, int blue, int alpha) { + int col_pos = col * 4; // Find column array position + if (!image || row < 0 || row >= (height - 1) || + col_pos < 0 || col_pos >= (width - 1) ) { + // invalid row / col + return false; + } + // Check pixel color + const unsigned char* pixels = GetPixels(row); + if (pixels[col_pos + 0] == red && + pixels[col_pos + 1] == green && + pixels[col_pos + 2] == blue && + pixels[col_pos + 3] == alpha) { + // Pixel color matches successfully + return true; + } else { + // Pixel color does not match + return false; + } +} + // Set Pixel Aspect Ratio void Frame::SetPixelRatio(int num, int den) { diff --git a/tests/FFmpegReader_Tests.cpp b/tests/FFmpegReader_Tests.cpp index 53563cac..454420a4 100644 --- a/tests/FFmpegReader_Tests.cpp +++ b/tests/FFmpegReader_Tests.cpp @@ -100,6 +100,10 @@ TEST(FFmpegReader_Check_Video_File) CHECK_EQUAL(0, (int)pixels[pixel_index + 2]); CHECK_EQUAL(255, (int)pixels[pixel_index + 3]); + // Check pixel function + CHECK_EQUAL(true, f->CheckPixel(10, 112, 21, 191, 0, 255)); + CHECK_EQUAL(false, f->CheckPixel(10, 112, 0, 0, 0, 0)); + // Get frame 1 f = r.GetFrame(2); @@ -113,6 +117,10 @@ TEST(FFmpegReader_Check_Video_File) CHECK_EQUAL(188, (int)pixels[pixel_index + 2]); CHECK_EQUAL(255, (int)pixels[pixel_index + 3]); + // Check pixel function + CHECK_EQUAL(true, f->CheckPixel(10, 112, 0, 96, 188, 255)); + CHECK_EQUAL(false, f->CheckPixel(10, 112, 0, 0, 0, 0)); + // Close reader r.Close(); } From 2be5e5e16845cdcf2f80383b1329808558e98c87 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Tue, 4 Jun 2019 13:42:46 -0500 Subject: [PATCH 13/19] Fixing crash on certain hardware accelerator modes (specifically decoder 2, device 0) --- src/FFmpegReader.cpp | 4 +--- src/examples/Example.cpp | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/FFmpegReader.cpp b/src/FFmpegReader.cpp index e8f135ce..6aa0938e 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -934,9 +934,7 @@ std::shared_ptr FFmpegReader::ReadStream(int64_t requested_frame) { // down processing considerably, but might be more stable on some systems. #pragma omp taskwait } - } else { - RemoveAVFrame(pFrame); - } + } } // Audio packet diff --git a/src/examples/Example.cpp b/src/examples/Example.cpp index 80339684..1e19f4d9 100644 --- a/src/examples/Example.cpp +++ b/src/examples/Example.cpp @@ -38,7 +38,7 @@ int main(int argc, char* argv[]) { Settings *s = Settings::Instance(); s->HARDWARE_DECODER = 2; // 1 VA-API, 2 NVDEC - s->HW_DE_DEVICE_SET = 1; + s->HW_DE_DEVICE_SET = 0; FFmpegReader r9("/home/jonathan/Videos/sintel_trailer-720p.mp4"); r9.Open(); From 438b2c333e1366e206957b18baef6e17d73f4a38 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 8 Jun 2019 12:21:56 -0500 Subject: [PATCH 14/19] Update Frame.cpp --- src/Frame.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Frame.cpp b/src/Frame.cpp index 27bf5f71..cacd91d9 100644 --- a/src/Frame.cpp +++ b/src/Frame.cpp @@ -481,7 +481,7 @@ const unsigned char* Frame::GetPixels(int row) } // Check a specific pixel color value (returns True/False) -bool Frame::CheckPixel(int row, int col, int red, int green, int blue, int alpha) { +bool Frame::CheckPixel(int row, int col, int red, int green, int blue, int alpha, int threshold) { int col_pos = col * 4; // Find column array position if (!image || row < 0 || row >= (height - 1) || col_pos < 0 || col_pos >= (width - 1) ) { @@ -490,10 +490,10 @@ bool Frame::CheckPixel(int row, int col, int red, int green, int blue, int alpha } // Check pixel color const unsigned char* pixels = GetPixels(row); - if (pixels[col_pos + 0] == red && - pixels[col_pos + 1] == green && - pixels[col_pos + 2] == blue && - pixels[col_pos + 3] == alpha) { + if (pixels[col_pos + 0] >= (red - threshold) && pixels[col_pos + 0] <= (red + threshold) && + pixels[col_pos + 0] >= (green - threshold) && pixels[col_pos + 0] <= (green + threshold) && + pixels[col_pos + 0] >= (blue - threshold) && pixels[col_pos + 0] <= (blue + threshold) && + pixels[col_pos + 0] >= (alpha - threshold) && pixels[col_pos + 0] <= (alpha + threshold)) { // Pixel color matches successfully return true; } else { From 2b308c6d5974a6cd643f2414b1f5b41285d831d2 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 8 Jun 2019 12:23:26 -0500 Subject: [PATCH 15/19] Update Frame.h --- include/Frame.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/Frame.h b/include/Frame.h index 72822188..6b682edb 100644 --- a/include/Frame.h +++ b/include/Frame.h @@ -250,7 +250,7 @@ namespace openshot const unsigned char* GetPixels(int row); /// Check a specific pixel color value (returns True/False) - bool CheckPixel(int row, int col, int red, int green, int blue, int alpha); + bool CheckPixel(int row, int col, int red, int green, int blue, int alpha, int threshold); /// Get height of image int GetHeight(); From 238e2d16d8a5a45716297415892dac50f2824761 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 8 Jun 2019 12:24:49 -0500 Subject: [PATCH 16/19] Update FFmpegReader_Tests.cpp --- tests/FFmpegReader_Tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/FFmpegReader_Tests.cpp b/tests/FFmpegReader_Tests.cpp index 454420a4..72f5462d 100644 --- a/tests/FFmpegReader_Tests.cpp +++ b/tests/FFmpegReader_Tests.cpp @@ -101,8 +101,8 @@ TEST(FFmpegReader_Check_Video_File) CHECK_EQUAL(255, (int)pixels[pixel_index + 3]); // Check pixel function - CHECK_EQUAL(true, f->CheckPixel(10, 112, 21, 191, 0, 255)); - CHECK_EQUAL(false, f->CheckPixel(10, 112, 0, 0, 0, 0)); + CHECK_EQUAL(true, f->CheckPixel(10, 112, 21, 191, 0, 255, 5)); + CHECK_EQUAL(false, f->CheckPixel(10, 112, 0, 0, 0, 0, 5)); // Get frame 1 f = r.GetFrame(2); From 3f926f45df8f7c2c30c0872346749beb47bf37f9 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 8 Jun 2019 12:45:13 -0500 Subject: [PATCH 17/19] Update FFmpegReader_Tests.cpp --- tests/FFmpegReader_Tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/FFmpegReader_Tests.cpp b/tests/FFmpegReader_Tests.cpp index 72f5462d..462a77c3 100644 --- a/tests/FFmpegReader_Tests.cpp +++ b/tests/FFmpegReader_Tests.cpp @@ -118,8 +118,8 @@ TEST(FFmpegReader_Check_Video_File) CHECK_EQUAL(255, (int)pixels[pixel_index + 3]); // Check pixel function - CHECK_EQUAL(true, f->CheckPixel(10, 112, 0, 96, 188, 255)); - CHECK_EQUAL(false, f->CheckPixel(10, 112, 0, 0, 0, 0)); + CHECK_EQUAL(true, f->CheckPixel(10, 112, 0, 96, 188, 255, 5)); + CHECK_EQUAL(false, f->CheckPixel(10, 112, 0, 0, 0, 0, 5)); // Close reader r.Close(); From 722d672f5838fc6799aa5faec0cc9da821750691 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sat, 8 Jun 2019 12:52:35 -0500 Subject: [PATCH 18/19] Update Frame.cpp --- src/Frame.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Frame.cpp b/src/Frame.cpp index cacd91d9..aa7c0d87 100644 --- a/src/Frame.cpp +++ b/src/Frame.cpp @@ -491,9 +491,9 @@ bool Frame::CheckPixel(int row, int col, int red, int green, int blue, int alpha // Check pixel color const unsigned char* pixels = GetPixels(row); if (pixels[col_pos + 0] >= (red - threshold) && pixels[col_pos + 0] <= (red + threshold) && - pixels[col_pos + 0] >= (green - threshold) && pixels[col_pos + 0] <= (green + threshold) && - pixels[col_pos + 0] >= (blue - threshold) && pixels[col_pos + 0] <= (blue + threshold) && - pixels[col_pos + 0] >= (alpha - threshold) && pixels[col_pos + 0] <= (alpha + threshold)) { + pixels[col_pos + 1] >= (green - threshold) && pixels[col_pos + 1] <= (green + threshold) && + pixels[col_pos + 2] >= (blue - threshold) && pixels[col_pos + 2] <= (blue + threshold) && + pixels[col_pos + 3] >= (alpha - threshold) && pixels[col_pos + 3] <= (alpha + threshold)) { // Pixel color matches successfully return true; } else { From 9378225d76dce86222c7aa6684bc916b6a292598 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Wed, 19 Jun 2019 22:21:50 -0400 Subject: [PATCH 19/19] Add -no-integrated-cpp for G++ < 9 --- CMakeLists.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2e3a49c3..03c5a761 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -75,6 +75,13 @@ Generating build files for OpenShot SO/API/ABI Version: ${SO_VERSION} ") +#### Work around a GCC < 9 bug with handling of _Pragma() in macros +#### See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55578 +if ((${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU") AND + (${CMAKE_CXX_COMPILER_VERSION} VERSION_LESS "9.0.0")) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -no-integrated-cpp") +endif() + #### Enable C++11 (for std::shared_ptr support) set(CMAKE_CXX_STANDARD 11) set(CMAKE_CXX_STANDARD_REQUIRED ON)