A big refactor of the Open() method on Readers, and the constructor of Readers. All resource consuming code has been moved into the Open() methods, so the Clips and Timeline can control them better.

include/Clip.h

@@ -94,9 +94,12 @@ namespace openshot {
 		/// Default Constructor
 		Clip();
 
-		/// Constructor with filepath
+		/// Constructor with filepath (reader is automatically created... by guessing file extensions)
 		Clip(string path);
 
+		/// Constructor with reader
+		Clip(FileReaderBase* reader);
+
 		/// Close the internal reader
 		void Close();
 
@@ -104,14 +107,19 @@ namespace openshot {
 		///
 		/// @returns The requested frame (containing the image)
 		/// @param[requested_frame] number The frame number that is requested.
-		Frame* GetFrame(int requested_frame);
+		Frame* GetFrame(int requested_frame) throw(ReaderClosed);
 
 		/// Map frame rate of this clip to a different frame rate
 		void MapFrames(Framerate fps, Pulldown_Method pulldown);
 
 		/// Open the internal reader
-		void Open();
+		void Open() throw(InvalidFile);
 
+		/// Set the current reader
+		void Reader(FileReaderBase* reader);
+
+		/// Get the current reader
+		FileReaderBase* Reader();
 
 		/// Get basic properties
 		float Position() { return position; } ///<Get position on timeline

include/Exceptions.h

@@ -167,6 +167,16 @@ namespace openshot {
 		virtual ~OutOfMemory() throw () {}
 	};
 
+	/// Exception when a reader is closed, and a frame is requested
+	class ReaderClosed : public BaseException
+	{
+	public:
+		string file_path;
+		ReaderClosed(string message, string file_path)
+			: BaseException(message), file_path(file_path) { }
+		virtual ~ReaderClosed() throw () {}
+	};
+
 	/// Exception when resample fails
 	class ResampleError : public BaseException
 	{

include/FFmpegReader.h

@@ -174,7 +174,7 @@ namespace openshot
 
 		/// Constructor for FFmpegReader.  This automatically opens the media file and loads
 		/// frame 1, or it throws one of the following exceptions.
-		FFmpegReader(string path) throw(InvalidFile, NoStreamsFound, InvalidCodec);
+		FFmpegReader(string path);
 
 		/// Close File
 		void Close();
@@ -183,10 +183,10 @@ namespace openshot
 		///
 		/// @returns The requested frame of video
 		/// @param[requested_frame] number The frame number that is requested.
-		Frame* GetFrame(int requested_frame);
+		Frame* GetFrame(int requested_frame) throw(ReaderClosed);
 
 		/// Open File - which is called by the constructor automatically
-		void Open();
+		void Open() throw(InvalidFile, NoStreamsFound, InvalidCodec);
 	};
 
 }

include/ImageReader.h

@@ -33,12 +33,13 @@ namespace openshot
 	private:
 		string path;
 		Frame* image_frame;
+		bool is_open;
 
 	public:
 
 		/// Constructor for ImageReader.  This automatically opens the media file and loads
 		/// frame 1, or it throws one of the following exceptions.
-		ImageReader(string path) throw(InvalidFile);
+		ImageReader(string path);
 
 		/// Close File
 		void Close();
@@ -48,10 +49,10 @@ namespace openshot
 		///
 		/// @returns The requested frame (containing the image)
 		/// @param[requested_frame] number The frame number that is requested.
-		Frame* GetFrame(int requested_frame) throw(InvalidFile);
+		Frame* GetFrame(int requested_frame) throw(ReaderClosed);
 
 		/// Open File - which is called by the constructor automatically
-		void Open();
+		void Open() throw(InvalidFile);
 	};
 
 }

include/Timeline.h

@@ -52,7 +52,7 @@ namespace openshot {
 		///
 		/// @returns The requested frame (containing the image)
 		/// @param[requested_frame] number The frame number that is requested.
-		Frame* GetFrame(int requested_frame);
+		Frame* GetFrame(int requested_frame) throw(ReaderClosed);
 
 		/// Close the reader (and any resources it was consuming)
 		void Close();

src/Clip.cpp

@@ -61,6 +61,13 @@ Clip::Clip()
 	init_settings();
 }
 
+// Constructor with reader
+Clip::Clip(FileReaderBase* reader)
+{
+	// set reader pointer
+	file_reader = reader;
+}
+
 // Constructor with filepath
 Clip::Clip(string path)
 {
@@ -72,7 +79,8 @@ Clip::Clip(string path)
 	transform(ext.begin(), ext.end(), ext.begin(), ::tolower);
 
 	// Determine if common video formats
-	if (ext=="avi" || ext=="mov" || ext=="mpg" || ext=="mpeg" || ext=="mp3" || ext=="mp4" || ext=="mts" || ext=="ogg" || ext=="wmv" || ext=="webm" || ext=="vob")
+	if (ext=="avi" || ext=="mov" || ext=="mkv" ||  ext=="mpg" || ext=="mpeg" || ext=="mp3" || ext=="mp4" || ext=="mts" ||
+		ext=="ogg" || ext=="wav" || ext=="wmv" || ext=="webm" || ext=="vob")
 	{
 		try
 		{
@@ -106,15 +114,32 @@ Clip::Clip(string path)
 		}
 	}
 
-	// Set some clip properties from the file reader
-	End(file_reader->info.duration);
+}
+
+/// Set the current reader
+void Clip::Reader(FileReaderBase* reader)
+{
+	// set reader pointer
+	file_reader = reader;
+}
+
+/// Get the current reader
+FileReaderBase* Clip::Reader()
+{
+	return file_reader;
 }
 
 // Open the internal reader
-void Clip::Open()
+void Clip::Open() throw(InvalidFile)
 {
 	if (file_reader)
+	{
+		// Open the reader
 		file_reader->Open();
+
+		// Set some clip properties from the file reader
+		End(file_reader->info.duration);
+	}
 }
 
 // Close the internal reader
@@ -125,7 +150,7 @@ void Clip::Close()
 }
 
 // Get an openshot::Frame object for a specific frame number of this reader.
-Frame* Clip::GetFrame(int requested_frame)
+Frame* Clip::GetFrame(int requested_frame) throw(ReaderClosed)
 {
 	// Adjust out of bounds frame number
 	if (requested_frame < 1)

src/FFmpegReader.cpp

@@ -2,7 +2,7 @@
 
 using namespace openshot;
 
-FFmpegReader::FFmpegReader(string path) throw(InvalidFile, NoStreamsFound, InvalidCodec)
+FFmpegReader::FFmpegReader(string path)
 	: last_frame(0), is_seeking(0), seeking_pts(0), seeking_frame(0),
 	  audio_pts_offset(99999), video_pts_offset(99999), working_cache(12), final_cache(24), path(path),
 	  is_video_seek(true), check_interlace(false), check_fps(false), enable_seek(true),
@@ -13,12 +13,6 @@ FFmpegReader::FFmpegReader(string path) throw(InvalidFile, NoStreamsFound, Inval
 
 	// Initialize FFMpeg, and register all formats and codecs
 	av_register_all();
-
-	// Open the file (if possible)
-	Open();
-
-	// Get 1st frame
-	GetFrame(1);
 }
 
 // Init a collection of software rescalers (thread safe)
@@ -46,7 +40,7 @@ void FFmpegReader::RemoveScalers()
 	image_rescalers.clear();
 }
 
-void FFmpegReader::Open()
+void FFmpegReader::Open() throw(InvalidFile, NoStreamsFound, InvalidCodec)
 {
 	// Open reader if not already open
 	if (!is_open)
@@ -136,6 +130,9 @@ void FFmpegReader::Open()
 
 		// Mark as "open"
 		is_open = true;
+
+		// Get 1st frame
+		GetFrame(1);
 	}
 }
 
@@ -279,8 +276,12 @@ void FFmpegReader::UpdateVideoInfo()
 
 }
 
-Frame* FFmpegReader::GetFrame(int requested_frame)
+Frame* FFmpegReader::GetFrame(int requested_frame) throw(ReaderClosed)
 {
+	// Check for open reader (or throw exception)
+	if (!is_open)
+		throw ReaderClosed("The FFmpegReader is closed.  Call Open() before calling this method.", path);
+
 	// Check the cache for this frame
 	if (final_cache.Exists(requested_frame))
 		// Return the cached frame

src/ImageReader.cpp

@@ -2,77 +2,91 @@
 
 using namespace openshot;
 
-ImageReader::ImageReader(string path) throw(InvalidFile) : path(path)
+ImageReader::ImageReader(string path) : path(path), is_open(false)
 {
 	// Init FileInfo struct (clear all values)
 	InitFileInfo();
-
-	// Open the file (if possible)
-	Open();
-
-	// Get 1st frame
-	GetFrame(1);
 }
 
 // Open image file
-void ImageReader::Open()
+void ImageReader::Open() throw(InvalidFile)
 {
-	// Attempt to open file
-	Magick::Image* source = NULL;
-	try
+	// Open reader if not already open
+	if (!is_open)
 	{
-		// load image
-		source = new Magick::Image(path);
+		// Attempt to open file
+		Magick::Image* source = NULL;
+		try
+		{
+			// load image
+			source = new Magick::Image(path);
+		}
+		catch (Magick::Exception e) {
+			// raise exception
+			throw InvalidFile("File could not be opened.", path);
+		}
+
+		// Create or get frame object
+		image_frame = new Frame(1, source->size().width(), source->size().height(), "#000000", 0, 2);
+
+		// Add Image data to frame
+		image_frame->AddImage(source);
+
+		// Update image properties
+		info.has_audio = false;
+		info.has_video = true;
+		info.file_size = source->fileSize();
+		info.vcodec = source->format();
+		info.width = source->size().width();
+		info.height = source->size().height();
+		info.pixel_ratio.num = 1;
+		info.pixel_ratio.den = 1;
+		info.duration = 60 * 60 * 24; // 24 hour duration
+		info.fps.num = 30;
+		info.fps.den = 1;
+		info.video_timebase.num = 1;
+		info.video_timebase.den = 30;
+		info.video_length = round(info.duration * info.fps.ToDouble());
+
+		// Calculate the DAR (display aspect ratio)
+		Fraction size(info.width * info.pixel_ratio.num, info.height * info.pixel_ratio.den);
+
+		// Reduce size fraction
+		size.Reduce();
+
+		// Set the ratio based on the reduced fraction
+		info.display_ratio.num = size.num;
+		info.display_ratio.den = size.den;
+
+		// Mark as "open"
+		is_open = true;
+
+		// Get 1st frame
+		GetFrame(1);
 	}
-	catch (Magick::Exception e) {
-		// raise exception
-		throw InvalidFile("File could not be opened.", path);
-	}
-
-	// Create or get frame object
-	image_frame = new Frame(1, source->size().width(), source->size().height(), "#000000", 0, 2);
-
-	// Add Image data to frame
-	image_frame->AddImage(source);
-
-
-	// Update image properties
-	info.has_audio = false;
-	info.has_video = true;
-	info.file_size = source->fileSize();
-	info.vcodec = source->format();
-	info.width = source->size().width();
-	info.height = source->size().height();
-	info.pixel_ratio.num = 1;
-	info.pixel_ratio.den = 1;
-	info.duration = 60 * 60 * 24; // 24 hour duration
-	info.fps.num = 30;
-	info.fps.den = 1;
-	info.video_timebase.num = 1;
-	info.video_timebase.den = 30;
-	info.video_length = round(info.duration * info.fps.ToDouble());
-
-	// Calculate the DAR (display aspect ratio)
-	Fraction size(info.width * info.pixel_ratio.num, info.height * info.pixel_ratio.den);
-
-	// Reduce size fraction
-	size.Reduce();
-
-	// Set the ratio based on the reduced fraction
-	info.display_ratio.num = size.num;
-	info.display_ratio.den = size.den;
 }
 
 // Close image file
 void ImageReader::Close()
 {
-	// Deallocate frame
-	delete image_frame;
+	// Close all objects, if reader is 'open'
+	if (is_open)
+	{
+		// Deallocate frame
+		delete image_frame;
+
+		// Mark as "closed"
+		is_open = false;
+	}
 }
 
 // Get an openshot::Frame object for a specific frame number of this reader.
-Frame* ImageReader::GetFrame(int requested_frame) throw(InvalidFile)
+Frame* ImageReader::GetFrame(int requested_frame) throw(ReaderClosed)
 {
+	// Check for open reader (or throw exception)
+	if (!is_open)
+		throw ReaderClosed("The ImageReader is closed.  Call Open() before calling this method.", path);
+
 	if (image_frame)
 		// Always return same frame (regardless of which frame number was requested)
 		return image_frame;

src/Timeline.cpp

@@ -52,7 +52,7 @@ void Timeline::Open()
 }
 
 // Get an openshot::Frame object for a specific frame number of this reader.
-Frame* Timeline::GetFrame(int requested_frame)
+Frame* Timeline::GetFrame(int requested_frame) throw(ReaderClosed)
 {
 	// Adjust out of bounds frame number
 	if (requested_frame < 1)

tests/Clip_Tests.cpp

@@ -23,6 +23,7 @@ TEST(Clip_Constructor)
 {
 	// Create a empty clip
 	Clip c1("../../src/examples/piano.wav");
+	c1.Open();
 
 	// Check basic settings
 	CHECK_EQUAL(ANCHOR_CANVAS, c1.anchor);
@@ -38,6 +39,7 @@ TEST(Clip_Basic_Gettings_and_Setters)
 {
 	// Create a empty clip
 	Clip c1;
+	c1.Open();
 
 	// Check basic settings
 	CHECK_EQUAL(ANCHOR_CANVAS, c1.anchor);

tests/FFmpegReader_Tests.cpp

@@ -6,14 +6,27 @@ using namespace openshot;
 
 TEST(FFmpegReader_Invalid_Path)
 {
+	// Create reader with invalid path
+	FFmpegReader r("");
+
 	// Check invalid path
-	CHECK_THROW(FFmpegReader r(""), InvalidFile);
+	CHECK_THROW(r.Open(), InvalidFile);
+}
+
+TEST(FFmpegReader_GetFrame_Before_Opening)
+{
+	// Create a reader
+	FFmpegReader r("../../src/examples/piano.wav");
+
+	// Check invalid path
+	CHECK_THROW(r.GetFrame(1), ReaderClosed);
 }
 
 TEST(FFmpegReader_Check_Audio_File)
 {
 	// Create a reader
 	FFmpegReader r("../../src/examples/piano.wav");
+	r.Open();
 
 	// Get frame 1
 	Frame *f = r.GetFrame(1);
@@ -41,6 +54,7 @@ TEST(FFmpegReader_Check_Video_File)
 {
 	// Create a reader
 	FFmpegReader r("../../src/examples/test.mp4");
+	r.Open();
 
 	// Get frame 1
 	Frame *f = r.GetFrame(1);