fix a number of memory leaks

Chris Kirmse · Chris Kirmse · commit 833fcb8e8e69 · 2019-05-08T15:02:33.000-07:00
- some were with libav functions
- same were due to non-virtual destructors
diff --git 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> frame) = 0;
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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());
-}
+}
diff --git 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;
-}
+}
diff --git 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<Frame> 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 <CriticalSection> lock(processingCriticalSection);
diff --git 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
@@ -120,7 +120,7 @@ void Frame::DeepCopy(const Frame& other)
 		wave_image = std::shared_ptr<QImage>(new QImage(*(other.wave_image)));
 }
 
-// Descructor
+// Destructor
 Frame::~Frame() {
 	// Clear all pointers
 	image.reset();
diff --git 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
@@ -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
@@ -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
@@ -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();
-}
+}
