Add _last_written to Writer, containing the last written frame and eyes

to each reel.  This is updated when things are popped off the queue, with
_state_mutex_held, and used in preference to the ones in ReelWriter
which were previously being updated during the time the _state_mutex
lock is unlocked in the body of Writer::thread().  This was not
thread safe (thanks, valgrind!)

diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc
index 49e09663ff1fed01d9caf5932eb90aeb6e63a158..06d20e7c8aeb2876ee67f3496cd0014995a7b085 100644 (file)
--- a/src/lib/reel_writer.cc
+++ b/src/lib/reel_writer.cc
@@ -71,8 +71,6 @@ ReelWriter::ReelWriter (
        )
        : _film (film)
        , _period (period)
-       , _last_written_video_frame (-1)
-       , _last_written_eyes (EYES_RIGHT)
        , _reel_index (reel_index)
        , _reel_count (reel_count)
        , _content_summary (content_summary)
@@ -266,12 +264,10 @@ ReelWriter::write (optional<Data> encoded, Frame frame, Eyes eyes)
        dcp::FrameInfo fin = _picture_asset_writer->write (encoded->data().get (), encoded->size());
        write_frame_info (frame, eyes, fin);
        _last_written[eyes] = encoded;
-       _last_written_video_frame = frame;
-       _last_written_eyes = eyes;
 }
 
 void
-ReelWriter::fake_write (Frame frame, Eyes eyes, int size)
+ReelWriter::fake_write (int size)
 {
        if (!_picture_asset_writer) {
                /* We're not writing any data */
@@ -279,8 +275,6 @@ ReelWriter::fake_write (Frame frame, Eyes eyes, int size)
        }
 
        _picture_asset_writer->fake_write (size);
-       _last_written_video_frame = frame;
-       _last_written_eyes = eyes;
 }
 
 void
@@ -296,8 +290,6 @@ ReelWriter::repeat_write (Frame frame, Eyes eyes)
                _last_written[eyes]->size()
                );
        write_frame_info (frame, eyes, fin);
-       _last_written_video_frame = frame;
-       _last_written_eyes = eyes;
 }
 
 void

diff --git a/src/lib/reel_writer.h b/src/lib/reel_writer.h
index d241c0fac16c067336179170050b99a427d66d4b..0b5a3dad59c321101b161fbdfb0eaf01d409dded 100644 (file)
--- a/src/lib/reel_writer.h
+++ b/src/lib/reel_writer.h
@@ -64,7 +64,7 @@ public:
                );
 
        void write (boost::optional<dcp::Data> encoded, Frame frame, Eyes eyes);
-       void fake_write (Frame frame, Eyes eyes, int size);
+       void fake_write (int size);
        void repeat_write (Frame frame, Eyes eyes);
        void write (boost::shared_ptr<const AudioBuffers> audio);
        void write (PlayerText text, TextType type, boost::optional<DCPTextTrack> track, dcpomatic::DCPTimePeriod period);
@@ -79,14 +79,6 @@ public:
                return _period;
        }
 
-       int last_written_video_frame () const {
-               return _last_written_video_frame;
-       }
-
-       Eyes last_written_eyes () const {
-               return _last_written_eyes;
-       }
-
        int first_nonexistant_frame () const {
                return _first_nonexistant_frame;
        }
@@ -109,9 +101,6 @@ private:
        int _first_nonexistant_frame;
        /** the data of the last written frame, if there is one */
        boost::optional<dcp::Data> _last_written[EYES_COUNT];
-       /** the index of the last written video frame within the reel */
-       int _last_written_video_frame;
-       Eyes _last_written_eyes;
        /** index of this reel within the DCP (starting from 0) */
        int _reel_index;
        /** number of reels in the DCP */

diff --git a/src/lib/writer.cc b/src/lib/writer.cc
index 6d1886406a61acb3e47c022cca489213c82def80..d346c4a4ff32eba5573ffe6c897f29301a02f101 100644 (file)
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -87,6 +87,8 @@ Writer::Writer (shared_ptr<const Film> film, weak_ptr<Job> j)
                _reels.push_back (ReelWriter (film, p, job, reel_index++, reels.size(), _film->content_summary(p)));
        }
 
+       _last_written.resize (reels.size());
+
        /* We can keep track of the current audio, subtitle and closed caption reels easily because audio
           and captions arrive to the Writer in sequence.  This is not so for video.
        */
@@ -300,7 +302,8 @@ Writer::write (shared_ptr<const AudioBuffers> audio, DCPTime const time)
        }
 }
 
-/** This must be called from Writer::thread() with an appropriate lock held */
+
+/** Caller must hold a lock on _state_mutex */
 bool
 Writer::have_sequenced_image_at_queue_head ()
 {
@@ -309,30 +312,41 @@ Writer::have_sequenced_image_at_queue_head ()
        }
 
        _queue.sort ();
-
        QueueItem const & f = _queue.front();
-       ReelWriter const & reel = _reels[f.reel];
+       return _last_written[f.reel].next(f);
+}
 
-       /* The queue should contain only EYES_LEFT/EYES_RIGHT pairs or EYES_BOTH */
 
-       if (f.eyes == EYES_BOTH) {
+bool
+Writer::LastWritten::next (QueueItem qi) const
+{
+       if (qi.eyes == EYES_BOTH) {
                /* 2D */
-               return f.frame == (reel.last_written_video_frame() + 1);
+               return qi.frame == (_frame + 1);
        }
 
        /* 3D */
 
-       if (reel.last_written_eyes() == EYES_LEFT && f.frame == reel.last_written_video_frame() && f.eyes == EYES_RIGHT) {
+       if (_eyes == EYES_LEFT && qi.frame == _frame && qi.eyes == EYES_RIGHT) {
                return true;
        }
 
-       if (reel.last_written_eyes() == EYES_RIGHT && f.frame == (reel.last_written_video_frame() + 1) && f.eyes == EYES_LEFT) {
+       if (_eyes == EYES_RIGHT && qi.frame == (_frame + 1) && qi.eyes == EYES_LEFT) {
                return true;
        }
 
        return false;
 }
 
+
+void
+Writer::LastWritten::update (QueueItem qi)
+{
+       _frame = qi.frame;
+       _eyes = qi.eyes;
+}
+
+
 void
 Writer::thread ()
 try
@@ -381,6 +395,7 @@ try
                /* Write any frames that we can write; i.e. those that are in sequence. */
                while (have_sequenced_image_at_queue_head ()) {
                        QueueItem qi = _queue.front ();
+                       _last_written[qi.reel].update (qi);
                        _queue.pop_front ();
                        if (qi.type == QueueItem::FULL && qi.encoded) {
                                --_queued_full_in_memory;
@@ -401,7 +416,7 @@ try
                                break;
                        case QueueItem::FAKE:
                                LOG_DEBUG_ENCODE (N_("Writer FAKE-writes %1"), qi.frame);
-                               reel.fake_write (qi.frame, qi.eyes, qi.size);
+                               reel.fake_write (qi.size);
                                ++_fake_written;
                                break;
                        case QueueItem::REPEAT:
@@ -430,7 +445,7 @@ try
                        DCPOMATIC_ASSERT (i != _queue.rend());
                        ++_pushed_to_disk;
                        /* For the log message below */
-                       int const awaiting = _reels[_queue.front().reel].last_written_video_frame() + 1;
+                       int const awaiting = _last_written[_queue.front().reel].frame() + 1;
                        lock.unlock ();
 
                        /* i is valid here, even though we don't hold a lock on the mutex,

diff --git a/src/lib/writer.h b/src/lib/writer.h
index d304133dcb98f2c3d380e191fda78d0d0eef61b1..d09b062649f94ea55f74ad563847f54118330772 100644 (file)
--- a/src/lib/writer.h
+++ b/src/lib/writer.h
@@ -151,6 +151,30 @@ private:
        int _maximum_frames_in_memory;
        unsigned int _maximum_queue_size;
 
+       class LastWritten
+       {
+       public:
+               LastWritten()
+                       : _frame(-1)
+                       , _eyes(EYES_RIGHT)
+               {}
+
+               /** @return true if qi is the next item after this one */
+               bool next (QueueItem qi) const;
+               void update (QueueItem qi);
+
+               int frame () const {
+                       return _frame;
+               }
+
+       private:
+               int _frame;
+               Eyes _eyes;
+       };
+
+       /** The last frame written to each reel */
+       std::vector<LastWritten> _last_written;
+
        /** number of FULL written frames */
        int _full_written;
        /** number of FAKE written frames */