Various multi-reel fixes.

* _video_frames_enqueued -> _position
* some int -> Frame replacements
* Call Writer::ref_write when there is nothing to encode / write
  for one frame.
* Assume Encoder::encode is called once per output video frame.
* Possibly correct some off-by-1 overlap bugs.
* Don't reset _last_written on FAKE write as I believe there is no need.

diff --git a/src/lib/encoder.cc b/src/lib/encoder.cc
index f7a66708ea3ab3925acc9cb94d538c4a7026a634..95a85f7e89a62c00718abf314bf6fce663369ab4 100644 (file)
--- a/src/lib/encoder.cc
+++ b/src/lib/encoder.cc
@@ -59,9 +59,7 @@ int const Encoder::_history_size = 25;
 Encoder::Encoder (shared_ptr<const Film> film, weak_ptr<Job> j, shared_ptr<Writer> writer)
        : _film (film)
        , _job (j)
-       , _video_frames_enqueued (0)
-       , _left_done (false)
-       , _right_done (false)
+       , _position (0)
        , _terminate_enqueue (false)
        , _terminate_encoding (false)
        , _writer (writer)
@@ -155,7 +153,7 @@ int
 Encoder::video_frames_out () const
 {
        boost::mutex::scoped_lock (_state_mutex);
-       return _video_frames_enqueued;
+       return _position;
 }
 
 /** Should be called when a frame has been encoded successfully.
@@ -174,9 +172,24 @@ Encoder::frame_done ()
        }
 }
 
-/** Called in order, so each time this is called the supplied frame is the one
- *  after the previous one.
+/** Called to start encoding of the next video frame in the DCP.  This is called in order,
+ *  so each time the supplied frame is the one after the previous one.
+ *  pv represents one video frame, and could be empty if there is nothing to encode
+ *  for this DCP frame.
  */
+void
+Encoder::encode (list<shared_ptr<PlayerVideo> > pv)
+{
+       if (pv.empty ()) {
+               _writer->ref_write (_position);
+       } else {
+               BOOST_FOREACH (shared_ptr<PlayerVideo> i, pv) {
+                       enqueue (i);
+               }
+       }
+       ++_position;
+}
+
 void
 Encoder::enqueue (shared_ptr<PlayerVideo> pv)
 {
@@ -210,22 +223,22 @@ Encoder::enqueue (shared_ptr<PlayerVideo> pv)
        */
        rethrow ();
 
-       if (_writer->can_fake_write (_video_frames_enqueued)) {
+       if (_writer->can_fake_write (_position)) {
                /* We can fake-write this frame */
-               _writer->fake_write (_video_frames_enqueued, pv->eyes ());
+               _writer->fake_write (_position, pv->eyes ());
                frame_done ();
        } else if (pv->has_j2k ()) {
                /* This frame already has JPEG2000 data, so just write it */
-               _writer->write (pv->j2k(), _video_frames_enqueued, pv->eyes ());
+               _writer->write (pv->j2k(), _position, pv->eyes ());
        } else if (_last_player_video && pv->same (_last_player_video)) {
-               _writer->repeat (_video_frames_enqueued, pv->eyes ());
+               _writer->repeat (_position, pv->eyes ());
        } else {
                /* Queue this new frame for encoding */
                LOG_TIMING ("add-frame-to-queue queue=%1", _queue.size ());
                _queue.push_back (shared_ptr<DCPVideo> (
                                          new DCPVideo (
                                                  pv,
-                                                 _video_frames_enqueued,
+                                                 _position,
                                                  _film->video_frame_rate(),
                                                  _film->j2k_bandwidth(),
                                                  _film->resolution(),
@@ -239,25 +252,6 @@ Encoder::enqueue (shared_ptr<PlayerVideo> pv)
                _empty_condition.notify_all ();
        }
 
-       switch (pv->eyes ()) {
-       case EYES_BOTH:
-               ++_video_frames_enqueued;
-               break;
-       case EYES_LEFT:
-               _left_done = true;
-               break;
-       case EYES_RIGHT:
-               _right_done = true;
-               break;
-       default:
-               break;
-       }
-
-       if (_left_done && _right_done) {
-               ++_video_frames_enqueued;
-               _left_done = _right_done = false;
-       }
-
        _last_player_video = pv;
 }
 

diff --git a/src/lib/encoder.h b/src/lib/encoder.h
index 76c87a9f7ad2ea57468a58408ea3ca663821340b..3d0597cfad5b533508e1dc28f907f0ff4a093e90 100644 (file)
--- a/src/lib/encoder.h
+++ b/src/lib/encoder.h
@@ -59,10 +59,10 @@ public:
        /** Called to indicate that a processing run is about to begin */
        void begin ();
 
-       /** Call with a frame of video.
-        *  @param f Video frame.
+       /** Called to pass in zero or more bits of video to be encoded
+        *  as the next DCP frame.
         */
-       void enqueue (boost::shared_ptr<PlayerVideo> f);
+       void encode (std::list<boost::shared_ptr<PlayerVideo> > f);
 
        /** Called when a processing run has finished */
        void end ();
@@ -72,6 +72,7 @@ public:
 
 private:
 
+       void enqueue (boost::shared_ptr<PlayerVideo> f);
        void frame_done ();
 
        void encoder_thread (boost::optional<ServerDescription>);
@@ -90,10 +91,8 @@ private:
        std::list<struct timeval> _time_history;
        /** Number of frames that we should keep history for */
        static int const _history_size;
-       /** Number of video frames enqueued so far */
-       int _video_frames_enqueued;
-       bool _left_done;
-       bool _right_done;
+       /** Current DCP frame index */
+       Frame _position;
 
        /* XXX: probably should be atomic */
        bool _terminate_enqueue;

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 6c3d959fa4488b360352e440babc91ce25b3556d..116410046e5f0eaa3b5acfc9eddb8aee533d1b0b 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -380,7 +380,7 @@ Player::get_video (DCPTime time, bool accurate)
 
        list<shared_ptr<Piece> > ov = overlaps<VideoContent> (
                time,
-               time + DCPTime::from_frames (1, _film->video_frame_rate ()) - DCPTime::delta()
+               time + DCPTime::from_frames (1, _film->video_frame_rate ())
                );
 
        list<shared_ptr<PlayerVideo> > pvf;

diff --git a/src/lib/player.h b/src/lib/player.h
index cd1423e9f89fd71d0a34a2cd745fcf2baab9ad2c..3106a3477abef0cd760a607004e78effe00e5fb5 100644 (file)
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -106,7 +106,7 @@ private:
                                continue;
                        }
 
-                       if ((*i)->content->position() <= to && (*i)->content->end() >= from) {
+                       if ((*i)->content->position() < to && (*i)->content->end() > from) {
                                overlaps.push_back (*i);
                        }
                }

diff --git a/src/lib/transcoder.cc b/src/lib/transcoder.cc
index 20185e8c9073aa1081440de839124fd3f75ff320..acff9c8965832a1af81739bffcd6376c38039e83 100644 (file)
--- a/src/lib/transcoder.cc
+++ b/src/lib/transcoder.cc
@@ -87,15 +87,8 @@ Transcoder::go ()
        }
 
        for (DCPTime t; t < length; t += frame) {
-               list<shared_ptr<PlayerVideo> > v = _player->get_video (t, true);
-               for (list<shared_ptr<PlayerVideo> >::const_iterator i = v.begin(); i != v.end(); ++i) {
-                       _encoder->enqueue (*i);
-               }
-
-               shared_ptr<AudioBuffers> audio = _player->get_audio (t, frame, true);
-               if (audio) {
-                       _writer->write (audio);
-               }
+               _encoder->encode (_player->get_video (t, true));
+               _writer->write (_player->get_audio (t, frame, true));
 
                if (non_burnt_subtitles) {
                        _writer->write (_player->get_subtitles (t, frame, true, false));

diff --git a/src/lib/writer.cc b/src/lib/writer.cc
index 736dba78e0cb3841c32ad015b246da244b71eda1..071cac4b6cea976ee71cabdf63e5acf09ddafb01 100644 (file)
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -90,6 +90,7 @@ Writer::Writer (shared_ptr<const Film> film, weak_ptr<Job> j)
        , _full_written (0)
        , _fake_written (0)
        , _repeat_written (0)
+       , _ref_written (0)
        , _pushed_to_disk (0)
 {
        /* Remove any old DCP */
@@ -178,8 +179,9 @@ Writer::~Writer ()
        terminate_thread (false);
 }
 
+/** @param frame Frame within the DCP */
 void
-Writer::write (Data encoded, int frame, Eyes eyes)
+Writer::write (Data encoded, Frame frame, Eyes eyes)
 {
        boost::mutex::scoped_lock lock (_state_mutex);
 
@@ -212,7 +214,7 @@ Writer::write (Data encoded, int frame, Eyes eyes)
 }
 
 void
-Writer::repeat (int frame, Eyes eyes)
+Writer::repeat (Frame frame, Eyes eyes)
 {
        boost::mutex::scoped_lock lock (_state_mutex);
 
@@ -239,7 +241,7 @@ Writer::repeat (int frame, Eyes eyes)
 }
 
 void
-Writer::fake_write (int frame, Eyes eyes)
+Writer::fake_write (Frame frame, Eyes eyes)
 {
        boost::mutex::scoped_lock lock (_state_mutex);
 
@@ -275,7 +277,29 @@ Writer::fake_write (int frame, Eyes eyes)
        _empty_condition.notify_all ();
 }
 
-/** This method is not thread safe */
+void
+Writer::ref_write (Frame frame)
+{
+       boost::mutex::scoped_lock lock (_state_mutex);
+
+       while (_queued_full_in_memory > _maximum_frames_in_memory) {
+               /* The queue is too big; wait until that is sorted out */
+               _full_condition.wait (lock);
+       }
+
+       QueueItem qi;
+       qi.type = QueueItem::REF;
+       qi.frame = frame;
+       qi.eyes = EYES_BOTH;
+       _queue.push_back (qi);
+
+       /* Now there's something to do: wake anything wait()ing on _empty_condition */
+       _empty_condition.notify_all ();
+}
+
+/** Write one video frame's worth of audio frames to the DCP.
+ *  This method is not thread safe.
+ */
 void
 Writer::write (shared_ptr<const AudioBuffers> audio)
 {
@@ -283,10 +307,17 @@ Writer::write (shared_ptr<const AudioBuffers> audio)
                return;
        }
 
-       _audio_reel->sound_asset_writer->write (audio->data(), audio->frames());
+       if (audio) {
+               _audio_reel->sound_asset_writer->write (audio->data(), audio->frames());
+       }
+
+       ++_audio_reel->written;
+
+       cout << "(written " << _audio_reel->written << "); period is " << _audio_reel->period.duration() << "\n";
 
-       /* SoundAsset's `frames' are video frames, not audio frames */
-       if (_audio_reel->sound_asset_writer->frames_written() >= _audio_reel->period.duration().frames_round (_film->video_frame_rate())) {
+       /* written is in video frames, not audio frames */
+       if (_audio_reel->written >= _audio_reel->period.duration().frames_round (_film->video_frame_rate())) {
+               cout << "NEXT AUDIO REEL!\n";
                ++_audio_reel;
        }
 }
@@ -416,10 +447,10 @@ try
                        case QueueItem::FAKE:
                                LOG_DEBUG_ENCODE (N_("Writer FAKE-writes %1"), qi.frame);
                                reel.picture_asset_writer->fake_write (qi.size);
-                               _last_written[qi.eyes].reset ();
                                ++_fake_written;
                                break;
                        case QueueItem::REPEAT:
+                       {
                                LOG_DEBUG_ENCODE (N_("Writer REPEAT-writes %1"), qi.frame);
                                dcp::FrameInfo fin = reel.picture_asset_writer->write (
                                        _last_written[qi.eyes]->data().get(),
@@ -429,6 +460,11 @@ try
                                ++_repeat_written;
                                break;
                        }
+                       case QueueItem::REF:
+                               LOG_DEBUG_ENCODE (N_("Writer REF-writes %1"), qi.frame);
+                               ++_ref_written;
+                               break;
+                       }
 
                        lock.lock ();
 
@@ -445,7 +481,7 @@ try
                                total *= 2;
                        }
                        if (total) {
-                               job->set_progress (float (_full_written + _fake_written + _repeat_written) / total);
+                               job->set_progress (float (_full_written + _fake_written + _repeat_written + _ref_written) / total);
                        }
                }
 
@@ -527,13 +563,16 @@ Writer::finish ()
        BOOST_FOREACH (Reel& i, _reels) {
 
                if (!i.picture_asset_writer->finalize ()) {
-               /* Nothing was written to the picture asset */
+                       /* Nothing was written to the picture asset */
                        i.picture_asset.reset ();
                }
 
                if (i.sound_asset_writer && !i.sound_asset_writer->finalize ()) {
                        /* Nothing was written to the sound asset */
+                       cout << "nothing written to reel @ " << i.period << "\n";
                        i.sound_asset.reset ();
+               } else {
+                       cout << "something written to reel @ " << i.period << "\n";
                }
 
                /* Hard-link any video asset file into the DCP */
@@ -718,7 +757,7 @@ Writer::finish ()
        dcp.write_xml (_film->interop () ? dcp::INTEROP : dcp::SMPTE, meta, signer);
 
        LOG_GENERAL (
-               N_("Wrote %1 FULL, %2 FAKE, %3 REPEAT, %4 pushed to disk"), _full_written, _fake_written, _repeat_written, _pushed_to_disk
+               N_("Wrote %1 FULL, %2 FAKE, %3 REPEAT, %4 REF, %5 pushed to disk"), _full_written, _fake_written, _repeat_written, _ref_written, _pushed_to_disk
                );
 }
 
@@ -791,22 +830,21 @@ Writer::check_existing_picture_asset (Reel& reel)
        fclose (info_file);
 }
 
-/** @param frame Frame index.
+/** @param frame Frame index within the whole DCP.
  *  @return true if we can fake-write this frame.
  */
 bool
-Writer::can_fake_write (int frame) const
+Writer::can_fake_write (Frame frame) const
 {
        /* We have to do a proper write of the first frame so that we can set up the JPEG2000
           parameters in the asset writer.
        */
 
-       /* XXX: need to correct frame to be relative to the reel, perhaps?
-          and clarify whether first_nonexistant_frame is relative to the start
-          of the DCP or of the reel.
-       */
+       Reel const & reel = video_reel (frame);
 
-       return (frame != 0 && frame < video_reel(frame).first_nonexistant_frame);
+       /* Make frame relative to the start of the reel */
+       frame -= reel.period.from.frames_floor (_film->video_frame_rate());
+       return (frame != 0 && frame < reel.first_nonexistant_frame);
 }
 
 void

diff --git a/src/lib/writer.h b/src/lib/writer.h
index 8f1e7127852116665e8f2e33b20b0029f3861052..95530a9d973587906af25a8ae290131bba5460a5 100644 (file)
--- a/src/lib/writer.h
+++ b/src/lib/writer.h
@@ -68,6 +68,7 @@ public:
                */
                FAKE,
                REPEAT,
+               REF,
        } type;
 
        /** encoded data for FULL */
@@ -76,6 +77,7 @@ public:
        int size;
        /** frame index */
        int frame;
+       /** eyes for FULL, FAKE and REPEAT */
        Eyes eyes;
 };
 
@@ -101,11 +103,12 @@ public:
 
        void start ();
 
-       bool can_fake_write (int) const;
+       bool can_fake_write (Frame) const;
 
-       void write (Data, int, Eyes);
-       void fake_write (int, Eyes);
-       void repeat (int, Eyes);
+       void write (Data, Frame, Eyes);
+       void fake_write (Frame, Eyes);
+       void ref_write (Frame);
+       void repeat (Frame, Eyes);
        void write (boost::shared_ptr<const AudioBuffers>);
        void write (PlayerSubtitles subs);
        void write (std::list<boost::shared_ptr<Font> > fonts);
@@ -120,11 +123,13 @@ private:
        public:
                Reel ()
                        : first_nonexistant_frame (0)
+                       , written (0)
                {}
 
                DCPTimePeriod period;
                /** the first frame index that does not already exist in our MXF */
                int first_nonexistant_frame;
+               Frame written;
 
                boost::shared_ptr<dcp::PictureAsset> picture_asset;
                boost::shared_ptr<dcp::PictureAssetWriter> picture_asset_writer;
@@ -178,6 +183,7 @@ private:
        /** number of FAKE written frames */
        int _fake_written;
        int _repeat_written;
+       int _ref_written;
        /** number of frames pushed to disk and then recovered
            due to the limit of frames to be held in memory.
        */