Tidying up; comments; fps in assets -> edit rate.

5 files changed, 102 insertions, 54 deletions

diff --git a/src/mxf_asset.cc b/src/mxf_asset.cc
index 7c75cb27..9b4cbf74 100644
--- a/src/mxf_asset.cc
+++ b/src/mxf_asset.cc
@@ -39,7 +39,7 @@ using namespace libdcp;
 MXFAsset::MXFAsset (string directory, string file_name)
 	: Asset (directory, file_name)
 	, _progress (0)
-	, _fps (0)
+	, _edit_rate (0)
 	, _entry_point (0)
 	, _intrinsic_duration (0)
 	, _duration (0)
@@ -47,29 +47,16 @@ MXFAsset::MXFAsset (string directory, string file_name)
 
 }
 
-MXFAsset::MXFAsset (string directory, string file_name, boost::signals2::signal<void (float)>* progress, int fps, int intrinsic_duration)
+MXFAsset::MXFAsset (string directory, string file_name, boost::signals2::signal<void (float)>* progress, int edit_rate, int intrinsic_duration)
 	: Asset (directory, file_name)
 	, _progress (progress)
-	, _fps (fps)
+	, _edit_rate (edit_rate)
 	, _entry_point (0)
 	, _intrinsic_duration (intrinsic_duration)
 	, _duration (intrinsic_duration)
 {
 	
 }
-
-void
-MXFAsset::set_entry_point (int e)
-{
-	_entry_point = e;
-}
-
-void
-MXFAsset::set_duration (int d)
-{
-	_duration = d;
-}
-
 void
 MXFAsset::fill_writer_info (ASDCP::WriterInfo* writer_info, string uuid)
 {
@@ -97,8 +84,8 @@ MXFAsset::equals (shared_ptr<const Asset> other, EqualityOptions, list<string>&
 		return false;
 	}
 
-	if (_fps != other_mxf->_fps) {
-		notes.push_back ("MXF frames per second differ");
+	if (_edit_rate != other_mxf->_edit_rate) {
+		notes.push_back ("MXF edit rates differ");
 		return false;
 	}
 	
@@ -114,9 +101,3 @@ MXFAsset::equals (shared_ptr<const Asset> other, EqualityOptions, list<string>&
 	
 	return true;
 }
-
-int
-MXFAsset::intrinsic_duration () const
-{
-	return _intrinsic_duration;
-}
diff --git a/src/mxf_asset.h b/src/mxf_asset.h
index ff43fecb..430e9157 100644
--- a/src/mxf_asset.h
+++ b/src/mxf_asset.h
@@ -30,31 +30,47 @@ namespace libdcp
 class MXFAsset : public Asset
 {
 public:
+	/** Construct an MXFAsset.
+	 *  This class will not write anything to disk in this constructor, but subclasses may.
+	 *
+	 *  @param directory Directory where MXF file is.
+	 *  @param file_name Name of MXF file.
+	 */
 	MXFAsset (std::string directory, std::string file_name);
 	
 	/** Construct an MXFAsset.
+	 *  This class will not write anything to disk in this constructor, but subclasses may.
+	 *
 	 *  @param directory Directory where MXF file is.
 	 *  @param file_name Name of MXF file.
-	 *  @param progress Signal to inform of progress.
-	 *  @param fps Frames per second.
+	 *  @param progress Signal to use to inform of progress, or 0.
+	 *  @param edit_rate Edit rate in frames per second (usually equal to the video frame rate).
 	 *  @param intrinsic_duration Duration of the whole asset in frames.
 	 */
-	MXFAsset (std::string directory, std::string file_name, boost::signals2::signal<void (float)>* progress, int fps, int intrinsic_duration);
+	MXFAsset (std::string directory, std::string file_name, boost::signals2::signal<void (float)>* progress, int edit_rate, int intrinsic_duration);
 
-	void set_entry_point (int e);
-	void set_duration (int d);
-
-	virtual bool equals (boost::shared_ptr<const Asset> other, EqualityOptions opt, std::list<std::string>& notes) const;
+	void set_entry_point (int e) {
+		_entry_point = e;
+	}
 	
-	int intrinsic_duration () const;
-	int frames_per_second () const {
-		return _fps;
+	void set_duration (int d) {
+		_duration = d;
 	}
 
 	void set_intrinsic_duration (int d) {
 		_intrinsic_duration = d;
 	}
 
+	virtual bool equals (boost::shared_ptr<const Asset> other, EqualityOptions opt, std::list<std::string>& notes) const;
+	
+	int intrinsic_duration () const {
+		return _intrinsic_duration;
+	}
+	
+	int edit_rate () const {
+		return _edit_rate;
+	}
+
 	/** Fill in a ADSCP::WriteInfo struct.
 	 *  @param w struct to fill in.
 	 *  @param uuid uuid to use.
@@ -63,10 +79,10 @@ public:
 
 protected:
 
-	/** Signal to emit to report progress */
+	/** Signal to emit to report progress, or 0 */
 	boost::signals2::signal<void (float)>* _progress;
-	/** Frames per second */
-	int _fps;
+	/** The edit rate; this is normally equal to the number of video frames per second */
+	int _edit_rate;
 	/** Start point to present in frames */
 	int _entry_point;
 	/** Total length in frames */
diff --git a/src/picture_asset.cc b/src/picture_asset.cc
index a7d5243a..a2f6b584 100644
--- a/src/picture_asset.cc
+++ b/src/picture_asset.cc
@@ -65,11 +65,11 @@ PictureAsset::write_to_cpl (ostream& s) const
 	s << "        <MainPicture>\n"
 	  << "          <Id>urn:uuid:" << _uuid << "</Id>\n"
 	  << "          <AnnotationText>" << _file_name << "</AnnotationText>\n"
-	  << "          <EditRate>" << _fps << " 1</EditRate>\n"
+	  << "          <EditRate>" << _edit_rate << " 1</EditRate>\n"
 	  << "          <IntrinsicDuration>" << _intrinsic_duration << "</IntrinsicDuration>\n"
 	  << "          <EntryPoint>" << _entry_point << "</EntryPoint>\n"
 	  << "          <Duration>" << _duration << "</Duration>\n"
-	  << "          <FrameRate>" << _fps << " 1</FrameRate>\n"
+	  << "          <FrameRate>" << _edit_rate << " 1</FrameRate>\n"
 	  << "          <ScreenAspectRatio>" << _size.width << " " << _size.height << "</ScreenAspectRatio>\n"
 	  << "        </MainPicture>\n";
 }
@@ -182,7 +182,7 @@ MonoPictureAsset::MonoPictureAsset (string directory, string mxf_name)
 
 	_size.width = desc.StoredWidth;
 	_size.height = desc.StoredHeight;
-	_fps = desc.EditRate.Numerator;
+	_edit_rate = desc.EditRate.Numerator;
 	assert (desc.EditRate.Denominator == 1);
 	_intrinsic_duration = desc.ContainerDuration;
 }
@@ -198,7 +198,7 @@ MonoPictureAsset::construct (boost::function<string (int)> get_path)
 	
 	ASDCP::JP2K::PictureDescriptor picture_desc;
 	j2k_parser.FillPictureDescriptor (picture_desc);
-	picture_desc.EditRate = ASDCP::Rational (_fps, 1);
+	picture_desc.EditRate = ASDCP::Rational (_edit_rate, 1);
 	
 	ASDCP::WriterInfo writer_info;
 	fill_writer_info (&writer_info, _uuid);
@@ -400,6 +400,9 @@ MonoPictureAsset::start_write ()
 	return shared_ptr<MonoPictureAssetWriter> (new MonoPictureAssetWriter (this));
 }
 
+/** @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* a)
 	: _frame_buffer (4 * Kumu::Megabyte)
 	, _asset (a)
@@ -412,13 +415,17 @@ MonoPictureAssetWriter::MonoPictureAssetWriter (MonoPictureAsset* a)
 void
 MonoPictureAssetWriter::write (uint8_t* data, int size)
 {
+	assert (!_finalized);
+	
 	if (ASDCP_FAILURE (_j2k_parser.OpenReadFrame (data, size, _frame_buffer))) {
 		throw MiscError ("could not parse J2K frame");
 	}
 
 	if (_frames_written == 0) {
+		/* This is our first frame; set up the writer */
+		
 		_j2k_parser.FillPictureDescriptor (_picture_descriptor);
-		_picture_descriptor.EditRate = ASDCP::Rational (_asset->frames_per_second(), 1);
+		_picture_descriptor.EditRate = ASDCP::Rational (_asset->edit_rate(), 1);
 	
 		MXFAsset::fill_writer_info (&_writer_info, _asset->uuid());
 		
diff --git a/src/picture_asset.h b/src/picture_asset.h
index dd69e352..ea558e7d 100644
--- a/src/picture_asset.h
+++ b/src/picture_asset.h
@@ -36,7 +36,24 @@ class StereoPictureFrame;
 class PictureAsset : public MXFAsset
 {
 public:
+	/** Construct a PictureAsset.
+	 *  This class will not write anything to disk in this constructor, but subclasses may.
+	 *  
+	 *  @param directory Directory where MXF file is.
+	 *  @param mxf_name Name of MXF file.
+	 */
 	PictureAsset (std::string directory, std::string mxf_name);
+
+	/** Construct a PictureAsset.
+	 *  This class will not write anything to disk in this constructor, but subclasses may.
+	 *
+	 *  @param directory Directory where MXF file is.
+	 *  @param mxf_name Name of MXF file.
+	 *  @param progress Signal to use to inform of progres, or 0.
+	 *  @param fps Video Frames per second.
+	 *  @param intrinsic_duration Duration of all the frames in the asset.
+	 *  @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, Size size);
 	
 	/** Write details of this asset to a CPL stream.
@@ -63,6 +80,16 @@ protected:
 
 class MonoPictureAsset;
 
+/** A helper class for writing to MonoPictureAssets 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 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.
+ *  The action of finalize() can't be done in MonoPictureAssetWriter's destructor as it may
+ *  throw an exception.
+ */
 class MonoPictureAssetWriter
 {
 public:
@@ -82,7 +109,9 @@ private:
 	ASDCP::WriterInfo _writer_info;
 	ASDCP::JP2K::PictureDescriptor _picture_descriptor;
 	MonoPictureAsset* _asset;
+	/** Number of picture frames written to the asset so far */
 	int _frames_written;
+	/** true if finalize() has been called */
 	bool _finalized;
 };
 
@@ -90,13 +119,14 @@ private:
 class MonoPictureAsset : public PictureAsset
 {
 public:
-	/** Construct a PictureAsset, generating the MXF from the JPEG2000 files.
+	/** Construct a MonoPictureAsset, generating the MXF from the JPEG2000 files.
 	 *  This may take some time; progress is indicated by emission of the Progress signal.
+	 *
 	 *  @param files Pathnames of JPEG2000 files, in frame order.
 	 *  @param directory Directory in which to create MXF file.
 	 *  @param mxf_name Name of MXF file to create.
 	 *  @param progress Signal to inform of progress.
-	 *  @param fps Frames per second.
+	 *  @param fps Video frames per second.
 	 *  @param intrinsic_duration Length of the whole asset in frames.
 	 *  @param size Size of images in pixels.
 	 */
@@ -110,13 +140,14 @@ public:
 		Size size
 		);
 
-	/** Construct a PictureAsset, generating the MXF from the JPEG2000 files.
+	/** Construct a MonoPictureAsset, generating the MXF from the JPEG2000 files.
 	 *  This may take some time; progress is indicated by emission of the Progress signal.
+	 *
 	 *  @param get_path Functor which returns a JPEG2000 file path for a given frame (frames counted from 0).
 	 *  @param directory Directory in which to create MXF file.
 	 *  @param mxf_name Name of MXF file to create.
 	 *  @param progress Signal to inform of progress.
-	 *  @param fps Frames per second.
+	 *  @param fps Video frames per second.
 	 *  @param intrinsic_duration Length of the whole asset in frames.
 	 *  @param size Size of images in pixels.
 	 */
@@ -130,10 +161,23 @@ public:
 		Size size
 		);
 
+	/** Construct a MonoPictureAsset, reading the MXF from disk.
+	 *  @param directory Directory that the MXF is in.
+	 *  @param mxf_name The filename of the MXF within `directory'.
+	 */
 	MonoPictureAsset (std::string directory, std::string mxf_name);
-	
+
+	/** Construct a MonoPictureAsset for progressive writing using
+	 *  start_write() and a MonoPictureAssetWriter.
+	 *
+	 *  @param directory Directory to put the MXF in.
+	 *  @param mxf_name Filename of the MXF within this directory.
+	 *  @param fps Video frames per second.
+	 *  @param size Size in pixels that the picture frames will be.
+	 */
 	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 ();
 
 	boost::shared_ptr<const MonoPictureFrame> get_frame (int n) const;
diff --git a/src/sound_asset.cc b/src/sound_asset.cc
index 89132f26..499f4112 100644
--- a/src/sound_asset.cc
+++ b/src/sound_asset.cc
@@ -92,7 +92,7 @@ SoundAsset::SoundAsset (string directory, string mxf_name)
 
 	_sampling_rate = desc.AudioSamplingRate.Numerator / desc.AudioSamplingRate.Denominator;
 	_channels = desc.ChannelCount;
-	_fps = desc.EditRate.Numerator;
+	_edit_rate = desc.EditRate.Numerator;
 	assert (desc.EditRate.Denominator == 1);
 	_intrinsic_duration = desc.ContainerDuration;
 }
@@ -117,10 +117,10 @@ SoundAsset::path_from_channel (Channel channel, vector<string> const & files)
 void
 SoundAsset::construct (boost::function<string (Channel)> get_path)
 {
-	ASDCP::Rational asdcp_fps (_fps, 1);
+	ASDCP::Rational asdcp_edit_rate (_edit_rate, 1);
 
  	ASDCP::PCM::WAVParser pcm_parser_channel[_channels];
-	if (pcm_parser_channel[0].OpenRead (get_path(LEFT).c_str(), asdcp_fps)) {
+	if (pcm_parser_channel[0].OpenRead (get_path(LEFT).c_str(), asdcp_edit_rate)) {
 		throw FileError ("could not open WAV file for reading", get_path(LEFT));
 	}
 	
@@ -128,7 +128,7 @@ SoundAsset::construct (boost::function<string (Channel)> get_path)
 	pcm_parser_channel[0].FillAudioDescriptor (audio_desc);
 	audio_desc.ChannelCount = 0;
 	audio_desc.BlockAlign = 0;
-	audio_desc.EditRate = asdcp_fps;
+	audio_desc.EditRate = asdcp_edit_rate;
 	audio_desc.AvgBps = audio_desc.AvgBps * _channels;
 
 	Channel channels[] = {
@@ -152,7 +152,7 @@ SoundAsset::construct (boost::function<string (Channel)> get_path)
 
 		string const path = get_path (channels[i]);
 		
-		if (ASDCP_FAILURE (pcm_parser_channel[i].OpenRead (path.c_str(), asdcp_fps))) {
+		if (ASDCP_FAILURE (pcm_parser_channel[i].OpenRead (path.c_str(), asdcp_edit_rate))) {
 			throw FileError ("could not open WAV file for reading", path);
 		}
 
@@ -227,7 +227,7 @@ SoundAsset::write_to_cpl (ostream& s) const
 	s << "        <MainSound>\n"
 	  << "          <Id>urn:uuid:" << _uuid << "</Id>\n"
 	  << "          <AnnotationText>" << _file_name << "</AnnotationText>\n"
-	  << "          <EditRate>" << _fps << " 1</EditRate>\n"
+	  << "          <EditRate>" << _edit_rate << " 1</EditRate>\n"
 	  << "          <IntrinsicDuration>" << _intrinsic_duration << "</IntrinsicDuration>\n"
 	  << "          <EntryPoint>" << _entry_point << "</EntryPoint>\n"
 	  << "          <Duration>" << _duration << "</Duration>\n"
@@ -328,7 +328,7 @@ SoundAssetWriter::SoundAssetWriter (SoundAsset* a)
 	, _frame_buffer_offset (0)
 {
 	/* Derived from ASDCP::Wav::SimpleWaveHeader::FillADesc */
-	_audio_desc.EditRate = ASDCP::Rational (_asset->frames_per_second(), 1);
+	_audio_desc.EditRate = ASDCP::Rational (_asset->edit_rate(), 1);
 	_audio_desc.AudioSamplingRate = ASDCP::Rational (_asset->sampling_rate(), 0);
 	_audio_desc.Locked = 0;
 	_audio_desc.ChannelCount = _asset->channels ();