Fix VideoDecoder::get_video() with 3D.

get_video() promises to return all video frames at the given time,
but this wasn't working for none-SBS-3D as it would be satisfied
when it got the first (left) frame.  Adjust get_video() to get all
required frames.

This showed up bugs in fill_both_eyes, whereby the from parameter
was ignored and the wrong things were done in some cases;
video_decoder_fill_test.cc tests this stuff.

diff --git a/src/lib/video_decoder.cc b/src/lib/video_decoder.cc
index 81991b88298f7c89c2571a656960abede3b1f9b7..cc6cbfc308d53ad065032f24ffc627668911d374 100644 (file)
--- a/src/lib/video_decoder.cc
+++ b/src/lib/video_decoder.cc
@@ -90,6 +90,8 @@ VideoDecoder::get (Frame frame, bool accurate)
                _parent->seek (ContentTime::from_frames (frame, _content->active_video_frame_rate()), accurate);
        }
 
+       unsigned int const frames_wanted = _content->video->frame_type() == VIDEO_FRAME_TYPE_2D ? 1 : 2;
+
        list<ContentVideo> dec;
 
        /* Now enough pass() calls should either:
@@ -104,7 +106,7 @@ VideoDecoder::get (Frame frame, bool accurate)
                bool no_data = false;
 
                while (true) {
-                       if (!decoded(frame).empty ()) {
+                       if (decoded(frame).size() == frames_wanted) {
                                /* We got what we want */
                                break;
                        }
@@ -131,10 +133,14 @@ VideoDecoder::get (Frame frame, bool accurate)
                }
 
        } else {
-               /* Any frame will do: use the first one that comes out of pass() */
-               while (_decoded.empty() && !_parent->pass (Decoder::PASS_REASON_VIDEO, accurate)) {}
-               if (!_decoded.empty ()) {
-                       dec.push_back (_decoded.front ());
+               /* Any frame(s) will do: use the first one(s) that comes out of pass() */
+               while (_decoded.size() < frames_wanted && !_parent->pass (Decoder::PASS_REASON_VIDEO, accurate)) {}
+               list<ContentVideo>::const_iterator i = _decoded.begin();
+               unsigned int j = 0;
+               while (i != _decoded.end() && j < frames_wanted) {
+                       dec.push_back (*i);
+                       ++i;
+                       ++j;
                }
        }
 
@@ -183,13 +189,8 @@ VideoDecoder::fill_one_eye (Frame from, Frame to, Eyes eye)
  *  adding both left and right eye frames.
  */
 void
-VideoDecoder::fill_both_eyes (Frame from, Frame to, Eyes eye)
+VideoDecoder::fill_both_eyes (Frame from_frame, Eyes from_eye, Frame to_frame, Eyes to_eye)
 {
-       if (to == 0 && eye == EYES_LEFT) {
-               /* Already OK */
-               return;
-       }
-
        /* Fill with black... */
        shared_ptr<const ImageProxy> filler_left_image (new RawImageProxy (_black_image));
        shared_ptr<const ImageProxy> filler_right_image (new RawImageProxy (_black_image));
@@ -211,21 +212,7 @@ VideoDecoder::fill_both_eyes (Frame from, Frame to, Eyes eye)
                }
        }
 
-       Frame filler_frame = from;
-       Eyes filler_eye = _decoded.empty() ? EYES_LEFT : _decoded.back().eyes;
-
-       if (_decoded.empty ()) {
-               filler_frame = 0;
-               filler_eye = EYES_LEFT;
-       } else if (_decoded.back().eyes == EYES_LEFT) {
-               filler_frame = _decoded.back().frame;
-               filler_eye = EYES_RIGHT;
-       } else if (_decoded.back().eyes == EYES_RIGHT) {
-               filler_frame = _decoded.back().frame + 1;
-               filler_eye = EYES_LEFT;
-       }
-
-       while (filler_frame != to || filler_eye != eye) {
+       while (from_frame != to_frame || from_eye != to_eye) {
 
 #ifdef DCPOMATIC_DEBUG
                test_gaps++;
@@ -233,18 +220,18 @@ VideoDecoder::fill_both_eyes (Frame from, Frame to, Eyes eye)
 
                _decoded.push_back (
                        ContentVideo (
-                               filler_eye == EYES_LEFT ? filler_left_image : filler_right_image,
-                               filler_eye,
-                               filler_eye == EYES_LEFT ? filler_left_part : filler_right_part,
-                               filler_frame
+                               from_eye == EYES_LEFT ? filler_left_image : filler_right_image,
+                               from_eye,
+                               from_eye == EYES_LEFT ? filler_left_part : filler_right_part,
+                               from_frame
                                )
                        );
 
-               if (filler_eye == EYES_LEFT) {
-                       filler_eye = EYES_RIGHT;
+               if (from_eye == EYES_LEFT) {
+                       from_eye = EYES_RIGHT;
                } else {
-                       filler_eye = EYES_LEFT;
-                       ++filler_frame;
+                       from_eye = EYES_LEFT;
+                       ++from_frame;
                }
        }
 }
@@ -265,6 +252,7 @@ VideoDecoder::give (shared_ptr<const ImageProxy> image, Frame frame)
        case VIDEO_FRAME_TYPE_2D:
                to_push.push_back (ContentVideo (image, EYES_BOTH, PART_WHOLE, frame));
                break;
+       case VIDEO_FRAME_TYPE_3D:
        case VIDEO_FRAME_TYPE_3D_ALTERNATE:
        {
                /* We receive the same frame index twice for 3D-alternate; hence we know which
@@ -297,40 +285,60 @@ VideoDecoder::give (shared_ptr<const ImageProxy> image, Frame frame)
           and the things we are about to push.
        */
 
-       optional<Frame> from;
-       optional<Frame> to;
+       optional<Frame> from_frame;
+       optional<Eyes> from_eye;
 
        if (_decoded.empty() && _last_seek_time && _last_seek_accurate) {
-               from = _last_seek_time->frames_round (_content->active_video_frame_rate ());
-               to = to_push.front().frame;
+               from_frame = _last_seek_time->frames_round (_content->active_video_frame_rate ());
+               from_eye = EYES_LEFT;
        } else if (!_decoded.empty ()) {
-               from = _decoded.back().frame + 1;
-               to = to_push.front().frame;
+               switch (_content->video->frame_type()) {
+               case VIDEO_FRAME_TYPE_2D:
+               case VIDEO_FRAME_TYPE_3D_LEFT:
+               case VIDEO_FRAME_TYPE_3D_RIGHT:
+                       from_frame = _decoded.back().frame + 1;
+                       break;
+               case VIDEO_FRAME_TYPE_3D:
+               case VIDEO_FRAME_TYPE_3D_LEFT_RIGHT:
+               case VIDEO_FRAME_TYPE_3D_TOP_BOTTOM:
+               case VIDEO_FRAME_TYPE_3D_ALTERNATE:
+                       /* Get the last frame that we have */
+                       from_frame = _decoded.back().frame;
+                       from_eye = _decoded.back().eyes;
+                       /* And increment */
+                       if (from_eye.get() == EYES_LEFT) {
+                               from_eye = EYES_RIGHT;
+                       } else {
+                               from_eye = EYES_LEFT;
+                               from_frame = from_frame.get() + 1;
+                       }
+               }
        }
 
        /* If we've pre-rolled on a seek we may now receive out-of-order frames
           (frames before the last seek time) which we can just ignore.
        */
 
-       if (from && to && from.get() > to.get()) {
+       if (from_frame && from_frame.get() > to_push.front().frame) {
                return;
        }
 
-       if (from) {
+       if (from_frame) {
                switch (_content->video->frame_type ()) {
                case VIDEO_FRAME_TYPE_2D:
-                       fill_one_eye (from.get(), to.get (), EYES_BOTH);
+                       fill_one_eye (from_frame.get(), to_push.front().frame, EYES_BOTH);
                        break;
+               case VIDEO_FRAME_TYPE_3D:
                case VIDEO_FRAME_TYPE_3D_LEFT_RIGHT:
                case VIDEO_FRAME_TYPE_3D_TOP_BOTTOM:
                case VIDEO_FRAME_TYPE_3D_ALTERNATE:
-                       fill_both_eyes (from.get(), to.get(), to_push.front().eyes);
+                       fill_both_eyes (from_frame.get(), from_eye.get(), to_push.front().frame, to_push.front().eyes);
                        break;
                case VIDEO_FRAME_TYPE_3D_LEFT:
-                       fill_one_eye (from.get(), to.get (), EYES_LEFT);
+                       fill_one_eye (from_frame.get(), to_push.front().frame, EYES_LEFT);
                        break;
                case VIDEO_FRAME_TYPE_3D_RIGHT:
-                       fill_one_eye (from.get(), to.get (), EYES_RIGHT);
+                       fill_one_eye (from_frame.get(), to_push.front().frame, EYES_RIGHT);
                        break;
                }
        }

diff --git a/src/lib/video_decoder.h b/src/lib/video_decoder.h
index 5053d32ebfe3a23aa60126e2ddee303dcd43a50c..7c2374dd8f154eaf370022c031824f04decd5b89 100644 (file)
--- a/src/lib/video_decoder.h
+++ b/src/lib/video_decoder.h
@@ -68,7 +68,7 @@ private:
 
        std::list<ContentVideo> decoded (Frame frame);
        void fill_one_eye (Frame from, Frame to, Eyes);
-       void fill_both_eyes (Frame from, Frame to, Eyes);
+       void fill_both_eyes (Frame from_frame, Eyes from_eyes, Frame to_frame, Eyes to_eyes);
 
        Decoder* _parent;
        boost::shared_ptr<const Content> _content;

diff --git a/test/video_decoder_fill_test.cc b/test/video_decoder_fill_test.cc
index da23b277558ba284d0a086f78d39a4105286610b..737bc883c62486b36f40516f27d77b66850e2d68 100644 (file)
--- a/test/video_decoder_fill_test.cc
+++ b/test/video_decoder_fill_test.cc
@@ -62,7 +62,7 @@ BOOST_AUTO_TEST_CASE (video_decoder_fill_test2)
        shared_ptr<ImageContent> c (new ImageContent (film, "test/data/simple_testcard_640x480.png"));
        ImageDecoder decoder (c, film->log());
 
-       decoder.video->fill_both_eyes (0, 4, EYES_LEFT);
+       decoder.video->fill_both_eyes (0, EYES_LEFT, 4, EYES_LEFT);
        BOOST_CHECK_EQUAL (decoder.video->_decoded.size(), 8);
        list<ContentVideo>::iterator i = decoder.video->_decoded.begin();
        for (int j = 0; j < 8; ++j) {
@@ -71,7 +71,8 @@ BOOST_AUTO_TEST_CASE (video_decoder_fill_test2)
                ++i;
        }
 
-       decoder.video->fill_both_eyes (0, 7, EYES_RIGHT);
+       decoder.video->_decoded.clear ();
+       decoder.video->fill_both_eyes (0, EYES_LEFT, 7, EYES_RIGHT);
        BOOST_CHECK_EQUAL (decoder.video->_decoded.size(), 15);
        i = decoder.video->_decoded.begin();
        for (int j = 0; j < 15; ++j) {
@@ -79,4 +80,14 @@ BOOST_AUTO_TEST_CASE (video_decoder_fill_test2)
                BOOST_CHECK_EQUAL (i->eyes, (j % 2) == 0 ? EYES_LEFT : EYES_RIGHT);
                ++i;
        }
+
+       decoder.video->_decoded.clear ();
+       decoder.video->fill_both_eyes (0, EYES_RIGHT, 7, EYES_RIGHT);
+       BOOST_CHECK_EQUAL (decoder.video->_decoded.size(), 14);
+       i = decoder.video->_decoded.begin();
+       for (int j = 0; j < 14; ++j) {
+               BOOST_CHECK_EQUAL (i->frame, (j + 1) / 2);
+               BOOST_CHECK_EQUAL (i->eyes, (j % 2) == 0 ? EYES_RIGHT : EYES_LEFT);
+               ++i;
+       }
 }