Simplify streaming API a bit.

5 files changed, 63 insertions, 43 deletions

diff --git a/src/picture_asset.cc b/src/picture_asset.cc
index c1805e6f..2aec187a 100644
--- a/src/picture_asset.cc
+++ b/src/picture_asset.cc
@@ -441,7 +441,7 @@ StereoPictureAsset::get_frame (int n) const
 	return shared_ptr<const StereoPictureFrame> (new StereoPictureFrame (path().string(), n));
 }
 
-shared_ptr<MonoPictureAssetWriter>
+shared_ptr<PictureAssetWriter>
 MonoPictureAsset::start_write (bool overwrite, MXFMetadata const & metadata)
 {
 	/* XXX: can't we use shared_ptr here? */
@@ -460,7 +460,7 @@ StereoPictureAsset::StereoPictureAsset (string directory, string mxf_name, int f
 
 }
 
-shared_ptr<StereoPictureAssetWriter>
+shared_ptr<PictureAssetWriter>
 StereoPictureAsset::start_write (bool overwrite, MXFMetadata const & metadata)
 {
 	/* XXX: can't we use shared_ptr here? */
diff --git a/src/picture_asset.h b/src/picture_asset.h
index 58d9c748..f1b14bd6 100644
--- a/src/picture_asset.h
+++ b/src/picture_asset.h
@@ -34,8 +34,7 @@ namespace libdcp
 
 class MonoPictureFrame;	
 class StereoPictureFrame;
-class MonoPictureAssetWriter;
-class StereoPictureAssetWriter;
+class PictureAssetWriter;
 
 /** @brief An asset made up of JPEG2000 files */
 class PictureAsset : public MXFAsset
@@ -60,6 +59,8 @@ public:
 	 *  @param size Size of video frame images in pixels.
 	 */
 	PictureAsset (std::string directory, std::string mxf_name, boost::signals2::signal<void (float)>* progress, int fps, int intrinsic_duration, bool encrypted, Size);
+
+	virtual boost::shared_ptr<PictureAssetWriter> start_write (bool, MXFMetadata const & metadata = MXFMetadata ()) = 0;
 	
 	bool equals (boost::shared_ptr<const Asset> other, EqualityOptions opt, boost::function<void (NoteType, std::string)> note) const;
 
@@ -154,7 +155,7 @@ public:
 	MonoPictureAsset (std::string directory, std::string mxf_name, int fps, Size size);
 
 	/** Start a progressive write to a MonoPictureAsset */
-	boost::shared_ptr<MonoPictureAssetWriter> start_write (bool, MXFMetadata const & metadata = MXFMetadata ());
+	boost::shared_ptr<PictureAssetWriter> start_write (bool, MXFMetadata const & metadata = MXFMetadata ());
 
 	boost::shared_ptr<const MonoPictureFrame> get_frame (int n) const;
 	bool equals (boost::shared_ptr<const Asset> other, EqualityOptions opt, boost::function<void (NoteType, std::string)> note) const;
@@ -183,7 +184,7 @@ public:
 	StereoPictureAsset (std::string directory, std::string mxf_name, int fps, Size size);
 
 	/** Start a progressive write to a StereoPictureAsset */
-	boost::shared_ptr<StereoPictureAssetWriter> start_write (bool, MXFMetadata const & metadata = MXFMetadata ());
+	boost::shared_ptr<PictureAssetWriter> start_write (bool, MXFMetadata const & metadata = MXFMetadata ());
 
 	boost::shared_ptr<const StereoPictureFrame> get_frame (int n) const;
 	bool equals (boost::shared_ptr<const Asset> other, EqualityOptions opt, boost::function<void (NoteType, std::string)> note) const;
diff --git a/src/picture_asset_writer.cc b/src/picture_asset_writer.cc
index 32e13a17..8ac76c9c 100644
--- a/src/picture_asset_writer.cc
+++ b/src/picture_asset_writer.cc
@@ -41,8 +41,9 @@ FrameInfo::write (ostream& s)
 }
 
 
-PictureAssetWriter::PictureAssetWriter (bool overwrite, MXFMetadata const & metadata)
-	: _frames_written (0)
+PictureAssetWriter::PictureAssetWriter (PictureAsset* asset, bool overwrite, MXFMetadata const & metadata)
+	: _asset (asset)
+	, _frames_written (0)
 	, _started (false)
 	, _finalized (false)
 	, _overwrite (overwrite)
@@ -51,47 +52,42 @@ PictureAssetWriter::PictureAssetWriter (bool overwrite, MXFMetadata const & meta
 	
 }
 
-struct MonoPictureAssetWriter::ASDCPState
+struct ASDCPStateBase
 {
-	ASDCPState()
+	ASDCPStateBase ()
 		: frame_buffer (4 * Kumu::Megabyte)
 	{}
 	
 	ASDCP::JP2K::CodestreamParser j2k_parser;
 	ASDCP::JP2K::FrameBuffer frame_buffer;
-	ASDCP::JP2K::MXFWriter mxf_writer;
 	ASDCP::WriterInfo writer_info;
 	ASDCP::JP2K::PictureDescriptor picture_descriptor;
 };
 
-struct StereoPictureAssetWriter::ASDCPState
+struct MonoPictureAssetWriter::ASDCPState : public ASDCPStateBase
+{
+	ASDCP::JP2K::MXFWriter mxf_writer;
+};
+
+struct StereoPictureAssetWriter::ASDCPState : public ASDCPStateBase
 {
-	ASDCPState()
-		: frame_buffer (4 * Kumu::Megabyte)
-	{}
-	
-	ASDCP::JP2K::CodestreamParser j2k_parser;
-	ASDCP::JP2K::FrameBuffer frame_buffer;
 	ASDCP::JP2K::MXFSWriter mxf_writer;
-	ASDCP::WriterInfo writer_info;
-	ASDCP::JP2K::PictureDescriptor picture_descriptor;
 };
 
 /** @param a Asset to write to.  `a' must not be deleted while
  *  this writer class still exists, or bad things will happen.
  */
-MonoPictureAssetWriter::MonoPictureAssetWriter (MonoPictureAsset* asset, bool overwrite, MXFMetadata const & metadata)
-	: PictureAssetWriter (overwrite, metadata)
+MonoPictureAssetWriter::MonoPictureAssetWriter (PictureAsset* asset, bool overwrite, MXFMetadata const & metadata)
+	: PictureAssetWriter (asset, overwrite, metadata)
 	, _state (new MonoPictureAssetWriter::ASDCPState)
-	, _asset (asset)
 {
 
 }
 
-StereoPictureAssetWriter::StereoPictureAssetWriter (StereoPictureAsset* asset, bool overwrite, MXFMetadata const & metadata)
-	: PictureAssetWriter (overwrite, metadata)
+StereoPictureAssetWriter::StereoPictureAssetWriter (PictureAsset* asset, bool overwrite, MXFMetadata const & metadata)
+	: PictureAssetWriter (asset, overwrite, metadata)
 	, _state (new StereoPictureAssetWriter::ASDCPState)
-	, _asset (asset)
+	, _next_eye (EYE_LEFT)
 {
 
 }
@@ -158,8 +154,12 @@ MonoPictureAssetWriter::write (uint8_t* data, int size)
 	return FrameInfo (before_offset, _state->mxf_writer.Tell() - before_offset, hash);
 }
 
+/** Write a frame for one eye.  Frames must be written left, then right, then left etc.
+ *  @param data JPEG2000 data.
+ *  @param size Size of data.
+ */
 FrameInfo
-StereoPictureAssetWriter::write (uint8_t* data, int size, Eye eye)
+StereoPictureAssetWriter::write (uint8_t* data, int size)
 {
 	assert (!_finalized);
 
@@ -174,15 +174,20 @@ StereoPictureAssetWriter::write (uint8_t* data, int size, Eye eye)
 	uint64_t const before_offset = _state->mxf_writer.Tell ();
 
 	string hash;
-	if (ASDCP_FAILURE (_state->mxf_writer.WriteFrame (
-				   _state->frame_buffer,
-				   (eye == EYE_LEFT) ? ASDCP::JP2K::SP_LEFT : ASDCP::JP2K::SP_RIGHT,
-				   0, 0, &hash)
+	if (ASDCP_FAILURE (
+		    _state->mxf_writer.WriteFrame (
+			    _state->frame_buffer,
+			    _next_eye == EYE_LEFT ? ASDCP::JP2K::SP_LEFT : ASDCP::JP2K::SP_RIGHT,
+			    0,
+			    0,
+			    &hash)
 		    )) {
 		
 		boost::throw_exception (MXFFileError ("error in writing video MXF", _asset->path().string()));
 	}
 
+	_next_eye = _next_eye == EYE_LEFT ? EYE_RIGHT : EYE_LEFT;
+
 	return FrameInfo (before_offset, _state->mxf_writer.Tell() - before_offset, hash);
 }
 
@@ -209,6 +214,7 @@ StereoPictureAssetWriter::fake_write (int size)
 		boost::throw_exception (MXFFileError ("error in writing video MXF", _asset->path().string()));
 	}
 
+	_next_eye = _next_eye == EYE_LEFT ? EYE_RIGHT : EYE_LEFT;
 	++_frames_written;
 }
 
diff --git a/src/picture_asset_writer.h b/src/picture_asset_writer.h
index 89651902..189ec422 100644
--- a/src/picture_asset_writer.h
+++ b/src/picture_asset_writer.h
@@ -27,8 +27,7 @@
 
 namespace libdcp {
 
-class MonoPictureAsset;	
-class StereoPictureAsset;	
+class PictureAsset;	
 
 struct FrameInfo
 {
@@ -50,6 +49,7 @@ struct FrameInfo
 class PictureAssetWriter : public boost::noncopyable
 {
 public:
+	virtual FrameInfo write (uint8_t *, int) = 0;
 	virtual void finalize () = 0;
 	virtual void fake_write (int) = 0;
 	
@@ -57,9 +57,14 @@ protected:
 	template <class P, class Q>
 	friend void start (PictureAssetWriter *, boost::shared_ptr<P>, Q *, uint8_t *, int);
 
-	PictureAssetWriter (bool, MXFMetadata const &);
+	PictureAssetWriter (PictureAsset *, bool, MXFMetadata const &);
+
+	PictureAsset* _asset;
 	
-	/** Number of picture frames written to the asset so far */
+	/** Number of picture frames written to the asset so far.  For stereo assets
+	 *  this will be incremented for each eye (i.e. there will be twice the number
+	 *  of frames as in a mono asset).
+	 */
 	int _frames_written;
 	bool _started;
 	/** true if finalize() has been called */
@@ -74,7 +79,7 @@ protected:
  *  Objects of this class can only be created with MonoPictureAsset::start_write().
  *
  *  Frames can be written to the MonoPictureAsset by calling write() with a JPEG2000 image
- *  (a verbatim .j2 file).  finalize() must be called after the last frame has been written.
+ *  (a verbatim .j2c file).  finalize() must be called after the last frame has been written.
  *  The action of finalize() can't be done in MonoPictureAssetWriter's destructor as it may
  *  throw an exception.
  */
@@ -88,7 +93,7 @@ public:
 private:
 	friend class MonoPictureAsset;
 
-	MonoPictureAssetWriter (MonoPictureAsset *, bool, MXFMetadata const &);
+	MonoPictureAssetWriter (PictureAsset *, bool, MXFMetadata const &);
 	void start (uint8_t *, int);
 
 	/* do this with an opaque pointer so we don't have to include
@@ -97,21 +102,29 @@ private:
 	   
 	struct ASDCPState;
 	boost::shared_ptr<ASDCPState> _state;
-
-	MonoPictureAsset* _asset;
 };
 
+/** A helper class for writing to StereoPictureAssets progressively (i.e. writing frame-by-frame,
+ *  rather than giving libdcp all the frames in one go).
+ *
+ *  Objects of this class can only be created with StereoPictureAsset::start_write().
+ *
+ *  Frames can be written to the MonoPictureAsset by calling write() with a JPEG2000 image
+ *  (a verbatim .j2c file).  finalize() must be called after the last frame has been written.
+ *  The action of finalize() can't be done in MonoPictureAssetWriter's destructor as it may
+ *  throw an exception.
+ */
 class StereoPictureAssetWriter : public PictureAssetWriter
 {
 public:
-	FrameInfo write (uint8_t *, int, Eye);
+	FrameInfo write (uint8_t *, int);
 	void fake_write (int size);
 	void finalize ();
 
 private:
 	friend class StereoPictureAsset;
 
-	StereoPictureAssetWriter (StereoPictureAsset *, bool, MXFMetadata const &);
+	StereoPictureAssetWriter (PictureAsset *, bool, MXFMetadata const &);
 	void start (uint8_t *, int);
 
 	/* do this with an opaque pointer so we don't have to include
@@ -121,7 +134,7 @@ private:
 	struct ASDCPState;
 	boost::shared_ptr<ASDCPState> _state;
 
-	StereoPictureAsset* _asset;
+	libdcp::Eye _next_eye;
 };
 
 }
diff --git a/test/recovery_test.cc b/test/recovery_test.cc
index f397b96b..df9a839c 100644
--- a/test/recovery_test.cc
+++ b/test/recovery_test.cc
@@ -42,7 +42,7 @@ BOOST_AUTO_TEST_CASE (recovery)
 	boost::filesystem::remove_all ("build/test/baz");
 	boost::filesystem::create_directories ("build/test/baz");
 	shared_ptr<libdcp::MonoPictureAsset> mp (new libdcp::MonoPictureAsset ("build/test/baz", "video1.mxf", 24, libdcp::Size (32, 32)));
-	shared_ptr<libdcp::MonoPictureAssetWriter> writer = mp->start_write (false);
+	shared_ptr<libdcp::PictureAssetWriter> writer = mp->start_write (false);
 
 	int written_size = 0;
 	for (int i = 0; i < 24; ++i) {