Refactored the cache class to only apply the max frames against "previous" frames, and allow unlimited number of future frames, to account for different types of files (some which have hundreds of tiny frames, which need large future cache).

include/Cache.h

@@ -20,23 +20,26 @@ namespace openshot {
 	 * recently accessed frames.
 	 *
 	 * Due to the high cost of decoding streams, once a frame is decoded, converted to RGB, and a Frame object is created,
-	 * it critical to keep these Frames cached for performance reasons.
+	 * it critical to keep these Frames cached for performance reasons.  However, the larger the cache, the more memory
+	 * is required.  You can set the max number of previous frames (relative to the current frame) to cache.  Frames that
+	 * come after the current frame are allowed to increase as much as needed (a requirement of the file readers).  A call
+	 * to GetFrame() sets the current frame of the cache.
 	 */
 	class Cache {
 	private:
-		int max_frames;				///< This is the max number of frames to cache
+		int max_frames;				///< This is the max number of "previous" frames to cache
 		map<int, Frame> frames;		///< This map holds the frame number and Frame objects
 		deque<int> frame_numbers;	///< This queue holds a sequential list of cached Frame numbers
 		int current_frame;			///< This is the last requested frame (used to dynamically adjust the max_frames)
 
-		/// Clean up cached frames that exceed the number in our max_frames variable
+		/// Clean up cached frames that exceed the max number of previous frames
 		void CleanUp();
 
 	public:
-		/// Default constructor, max frames to cache is 20
+		/// Default constructor, max previous frames to cache is 20
 		Cache();
 
-		/// Constructor that sets the max frames to cache
+		/// Constructor that sets the max previous frames to cache
 		Cache(int max_frames);
 
 		/// Add a Frame to the cache
@@ -66,8 +69,14 @@ namespace openshot {
 		/// Count the frames in the queue
 		int Count();
 
+		/// Set current frame number (used to determine which previous frames to delete)
+		void SetCurrentFrame(int frame_number) { current_frame = frame_number; CleanUp(); };
+
+		/// Get the current frame number (used to determine which previous frames to delete)
+		int GetCurrentFrame() { return current_frame; };
+
 		/// Set maximum frames to a different amount
-		void SetMaxFrames(int max_frames) { max_frames = max_frames; };
+		void SetMaxFrames(int number_of_frames) { max_frames = number_of_frames; };
 
 		/// Gets the maximum frames value
 		int GetMaxFrames() { return max_frames; };

src/Cache.cpp

@@ -10,10 +10,10 @@ using namespace std;
 using namespace openshot;
 
 // Default constructor, max frames to cache is 20
-Cache::Cache() : max_frames(20) { };
+Cache::Cache() : max_frames(20), current_frame(0) { };
 
 // Constructor that sets the max frames to cache
-Cache::Cache(int max_frames) : max_frames(max_frames) { };
+Cache::Cache(int max_frames) : max_frames(max_frames), current_frame(0) { };
 
 // Add a Frame to the cache
 void Cache::Add(int frame_number, Frame frame)
@@ -52,9 +52,6 @@ Frame Cache::GetFrame(int frame_number)
 	// Does frame exists in cache?
 	if (Exists(frame_number))
 	{
-		// Set current frame
-		current_frame = frame_number;
-
 		// return the Frame object
 		return frames[frame_number];
 	}
@@ -86,19 +83,19 @@ void Cache::Remove(int frame_number)
 	Frame f = GetFrame(frame_number);
 
 	// Loop through frame numbers
-	 deque<int>::iterator itr;
-	 for(itr = frame_numbers.begin(); itr != frame_numbers.end(); ++itr)
-	 {
-		 if (*itr == frame_number)
-		 {
-			 // erase frame number
-			 frame_numbers.erase(itr);
-			 break;
-		 }
-	 }
+	deque<int>::iterator itr;
+	for(itr = frame_numbers.begin(); itr != frame_numbers.end(); ++itr)
+	{
+		if (*itr == frame_number)
+		{
+			// erase frame number
+			frame_numbers.erase(itr);
+			break;
+		}
+	}
 
-	 // Remove frame from map
-	 frames.erase(frame_number);
+	// Remove frame from map
+	frames.erase(frame_number);
 }
 
 // Clear the cache of all frames
@@ -121,14 +118,39 @@ int Cache::Count()
 // Clean up cached frames that exceed the number in our max_frames variable
 void Cache::CleanUp()
 {
-	// check against max size
-	if (frame_numbers.size() > max_frames)
-	{
-		// Remove the oldest frame
-		int frame_to_remove = frame_numbers.back();
+	// Count previous frames (relative to the current frame), and determine the smallest frame number
+	// Loop through frame numbers
+	deque<int>::iterator itr;
+	int previous_frames = 0;
+	int smallest_frame = -1;
+	for (itr = frame_numbers.begin(); itr != frame_numbers.end(); ++itr) {
+		if (*itr < current_frame)
+			previous_frames++;
 
-		// Remove frame_number and frame
-		Remove(frame_to_remove);
+		if (*itr < smallest_frame || smallest_frame == -1)
+			smallest_frame = *itr;
+	}
+
+	// check against max size
+	if (previous_frames > max_frames) {
+		// Get the difference
+		int diff = previous_frames - max_frames;
+		int removed_count = 0;
+
+		// Loop through frames and remove the oldest
+		for (int x = smallest_frame; x < current_frame; x++) {
+
+			// Does frame exist?
+			if (Exists(x)) {
+				// Remove the frame, increment count
+				Remove(x);
+				removed_count++;
+			}
+
+			// Break after the correct # has been removed
+			if (removed_count == diff)
+				break;
+		}
 	}
 }
 
@@ -168,4 +190,3 @@ void Cache::DisplayAndClear()
 
 
 
-

src/FFmpegReader.cpp

@@ -4,7 +4,7 @@ using namespace openshot;
 
 FFmpegReader::FFmpegReader(string path) throw(InvalidFile, NoStreamsFound, InvalidCodec)
 	: last_video_frame(0), last_audio_frame(0), is_seeking(0), seeking_pts(0), seeking_frame(0),
-	  audio_pts_offset(99999), video_pts_offset(99999), working_cache(130), final_cache(130), path(path),
+	  audio_pts_offset(99999), video_pts_offset(99999), working_cache(30), final_cache(30), path(path),
 	  is_video_seek(true), check_interlace(false), check_fps(false), init_settings(false),
 	  enable_seek(true) { // , resampleCtx(NULL)
 
@@ -374,8 +374,14 @@ Frame FFmpegReader::ReadStream(int requested_frame)
 
 	// Return requested frame (if found)
 	if (final_cache.Exists(requested_frame))
+	{
+		// Set cache's current frame
+		working_cache.SetCurrentFrame(requested_frame);
+		final_cache.SetCurrentFrame(requested_frame);
+
 		// Return prepared frame
 		return final_cache.GetFrame(requested_frame);
+	}
 	else
 		// Return blank frame
 		return CreateFrame(requested_frame);

src/Main.cpp

@@ -14,9 +14,9 @@ void FrameReady(int number)
 int main()
 {
 
-	//	openshot::FFmpegReader r("/home/jonathan/apps/libopenshot/src/examples/test.mp4");
-	//	openshot::FFmpegReader r("/home/jonathan/apps/libopenshot/src/examples/test1.mp4");
-	//	openshot::FFmpegReader r("/home/jonathan/apps/libopenshot/src/examples/piano.wav");
+	//	openshot::FFmpegReader r("/home/jonathan/Apps/libopenshot/src/examples/test.mp4");
+	//	openshot::FFmpegReader r("/home/jonathan/Apps/libopenshot/src/examples/test1.mp4");
+	//	openshot::FFmpegReader r("/home/jonathan/Apps/libopenshot/src/examples/piano.wav");
 	//	openshot::FFmpegReader r("/home/jonathan/Videos/sintel-1024-stereo.mp4");
 	//	openshot::FFmpegReader r("/home/jonathan/Videos/00001.mts");
 	//	openshot::FFmpegReader r("/home/jonathan/Videos/sintel_trailer-720p.mp4");
@@ -32,7 +32,7 @@ int main()
 		for (int repeat = 0; repeat <= 100; repeat++)
 		{
 			cout << "----------- REPEAT READER " << repeat << " ---------------" << endl;
-			for (int frame = 1; frame <= 100; frame++)
+			for (int frame = 300; frame <= 400; frame++)
 			{
 				Frame f = r.GetFrame(frame);
 				//f.Play();

tests/Cache_Tests.cpp

@@ -16,39 +16,88 @@ TEST(Cache_Default_Constructor)
 		c.Add(i, Frame());
 	}
 
-	// Cache should only have 20 items (since 20 is the max size in the default constructor)
-	CHECK_EQUAL(20, c.Count());
+	CHECK_EQUAL(30, c.Count()); // Cache should still have all 30 items, because the current frame is 0
+	CHECK_EQUAL(20, c.GetMaxFrames()); // Max frames should default to 20
 }
 
 TEST(Cache_Max_Frames_Constructor)
 {
-	// Create cache object (with a max of 10 items)
-	Cache c(10);
+	// Create cache object (with a max of 5 previous items)
+	Cache c(5);
 
 	// Loop 20 times
-	for (int i = 0; i <= 20; i++)
+	for (int i = 20; i > 0; i--)
 	{
 		// Add blank frame to the cache
 		c.Add(i, Frame());
 	}
 
-	// Cache should only have 10 items
-	CHECK_EQUAL(10, c.Count());
+	// Cache should have all 20 (since current frame is 0)
+	CHECK_EQUAL(20, c.Count());
 
-	// Check which 10 items the cache kept (should be the 10 most recent)
+	// Set current frame to 15
+	c.SetCurrentFrame(15);
+
+	// Since current frame is 15, only 5 previous items are allowed (i.e. frame 10 to 20)
+	CHECK_EQUAL(11, c.Count());
+
+	// Add 10 frames again
+	for (int i = 9; i > 0; i--)
+	{
+		// Add blank frame to the cache
+		c.Add(i, Frame());
+	}
+
+	// Count should still be 11, since all these frames were previous to 15
+	CHECK_EQUAL(11, c.Count());
+
+	// Check which 10 items the cache kept
 	CHECK_EQUAL(false, c.Exists(1));
 	CHECK_EQUAL(false, c.Exists(5));
 	CHECK_EQUAL(false, c.Exists(9));
-	CHECK_EQUAL(false, c.Exists(10));
+	CHECK_EQUAL(true, c.Exists(10));
 	CHECK_EQUAL(true, c.Exists(11));
 	CHECK_EQUAL(true, c.Exists(15));
 	CHECK_EQUAL(true, c.Exists(19));
 	CHECK_EQUAL(true, c.Exists(20));
+
+	// Since we're not adding any new frames, the count should decrease by 1
+	c.SetCurrentFrame(16);
+	CHECK_EQUAL(10, c.Count());
+
+	// Since we're not adding any new frames, the count should decrease by 1
+	c.SetCurrentFrame(17);
+	CHECK_EQUAL(9, c.Count());
+
+	// Since we're not adding any new frames, the count should decrease by 1
+	c.SetCurrentFrame(18);
+	CHECK_EQUAL(8, c.Count());
+
+	// Add first 18 frames again
+	for (int i = 18; i > 0; i--)
+	{
+		// Add blank frame to the cache
+		c.Add(i, Frame());
+	}
+
+	// Count should be 8
+	CHECK_EQUAL(8, c.Count());
+
+	// Increase frames to 50
+	for (int i = 1; i <= 50; i++)
+	{
+		// Add blank frame to the cache
+		c.Add(i, Frame());
+	}
+
+	// Count should be 38 (5 previous to 18 + all frames after 18)
+	CHECK_EQUAL(38, c.Count());
+
 }
 
 TEST(Cache_Clear)
 {
-	// Create cache object (with a max of 10 items)
+	// Create cache object (with a max of 10 previous items)
 	Cache c(10);
 
 	// Loop 10 times
@@ -86,7 +135,7 @@ TEST(Cache_Add_Duplicate_Frames)
 
 TEST(Cache_Check_If_Frame_Exists)
 {
-	// Create cache object (with a max of 10 items)
+	// Create cache object (with a max of 5 items)
 	Cache c(5);
 
 	// Loop 5 times
@@ -198,3 +247,52 @@ TEST(Cache_Remove)
 	// Check if count is 1
 	CHECK_EQUAL(1, c.Count());
 }
+
+TEST(Cache_Current_Frame)
+{
+	// Create cache object
+	Cache c;
+
+	// Loop 20 times
+	for (int i = 0; i < 20; i++)
+	{
+		// Add blank frame to the cache
+		c.Add(i, Frame());
+	}
+
+	CHECK_EQUAL(0, c.GetCurrentFrame()); // Cache defaults current frame is 0
+
+	// Set current frame
+	c.SetCurrentFrame(10);
+
+	CHECK_EQUAL(10, c.GetCurrentFrame()); // Cache should now be on frame 10
+
+	// Set current frame
+	c.SetCurrentFrame(20);
+
+	CHECK_EQUAL(20, c.GetCurrentFrame()); // Cache should now be on frame 20
+}
+
+TEST(Cache_Set_Max_Frames)
+{
+	// Create cache object
+	Cache c;
+
+	// Loop 20 times
+	for (int i = 0; i < 20; i++)
+	{
+		// Add blank frame to the cache
+		c.Add(i, Frame());
+	}
+
+	CHECK_EQUAL(20, c.GetMaxFrames()); // Cache defaults max frames to 20
+
+	// Set max frames
+	c.SetMaxFrames(10);
+	CHECK_EQUAL(10, c.GetMaxFrames());
+
+	// Set max frames
+	c.SetMaxFrames(30);
+	CHECK_EQUAL(30, c.GetMaxFrames());
+}
+