Add some more tests; improve seeking.
authorCarl Hetherington <cth@carlh.net>
Sat, 21 Dec 2013 18:44:52 +0000 (18:44 +0000)
committerCarl Hetherington <cth@carlh.net>
Sat, 21 Dec 2013 18:44:52 +0000 (18:44 +0000)
src/lib/ffmpeg_decoder.cc
src/lib/ffmpeg_decoder.h
src/lib/player.cc
test/ffmpeg_seek_test.cc
test/repeat_frame_test.cc [new file with mode: 0644]
test/seek_zero_test.cc [new file with mode: 0644]
test/skip_frame_test.cc [new file with mode: 0644]
test/wscript

index 1cf9bd92dc91dfd8bcf0e72aeb3c721b0377ceb9..80c3d9906b48188d2057f34007f632ac4671c9dc 100644 (file)
@@ -296,11 +296,11 @@ FFmpegDecoder::bytes_per_audio_sample () const
 }
 
 int
-FFmpegDecoder::minimal_run (boost::function<bool (ContentTime, ContentTime, int)> finished)
+FFmpegDecoder::minimal_run (boost::function<bool (optional<ContentTime>, optional<ContentTime>, int)> finished)
 {
        int frames_read = 0;
-       ContentTime last_video = 0;
-       ContentTime last_audio = 0;
+       optional<ContentTime> last_video;
+       optional<ContentTime> last_audio;
 
        while (!finished (last_video, last_audio, frames_read)) {
                int r = av_read_frame (_format_context, &_packet);
@@ -353,9 +353,9 @@ FFmpegDecoder::minimal_run (boost::function<bool (ContentTime, ContentTime, int)
 }
 
 bool
-FFmpegDecoder::seek_overrun_finished (ContentTime seek, ContentTime last_video, ContentTime last_audio) const
+FFmpegDecoder::seek_overrun_finished (ContentTime seek, optional<ContentTime> last_video, optional<ContentTime> last_audio) const
 {
-       return last_video >= seek || last_audio >= seek;
+       return (last_video && last_video.get() >= seek) || (last_audio && last_audio.get() >= seek);
 }
 
 bool
@@ -369,16 +369,11 @@ FFmpegDecoder::seek_and_flush (ContentTime t)
 {
        int64_t const initial_v = ((double (t) / TIME_HZ) - _video_pts_offset) /
                av_q2d (_format_context->streams[_video_stream]->time_base);
+       
+       int64_t const initial_a = ((double (t) / TIME_HZ) - _audio_pts_offset) /
+               av_q2d (_ffmpeg_content->audio_stream()->stream(_format_context)->time_base);
 
-       av_seek_frame (_format_context, _video_stream, initial_v, AVSEEK_FLAG_BACKWARD);
-
-       shared_ptr<FFmpegAudioStream> as = _ffmpeg_content->audio_stream ();
-       if (as) {
-               int64_t initial_a = ((double (t) / TIME_HZ) - _audio_pts_offset) /
-                       av_q2d (as->stream(_format_context)->time_base);
-
-               av_seek_frame (_format_context, as->index (_format_context), initial_a, AVSEEK_FLAG_BACKWARD);
-       }
+       av_seek_frame (_format_context, _video_stream, min (initial_v, initial_a), AVSEEK_FLAG_BACKWARD);
 
        avcodec_flush_buffers (video_codec_context());
        if (audio_codec_context ()) {
@@ -409,8 +404,8 @@ FFmpegDecoder::seek (ContentTime time, bool accurate)
 
        seek_and_flush (initial_seek);
 
-       if (time == 0 || !accurate) {
-               /* We're already there, or we're as close as we need to be */
+       if (!accurate) {
+               /* That'll do */
                return;
        }
 
index e50b248cff35992ad3da52c37e6b9fb97ecf12d5..55b624bd60e1df843fbfb7eb0aacd7ef9c9746a6 100644 (file)
@@ -71,9 +71,9 @@ private:
        void maybe_add_subtitle ();
        boost::shared_ptr<AudioBuffers> deinterleave_audio (uint8_t** data, int size);
 
-       bool seek_overrun_finished (ContentTime, ContentTime, ContentTime) const;
+       bool seek_overrun_finished (ContentTime, boost::optional<ContentTime>, boost::optional<ContentTime>) const;
        bool seek_final_finished (int, int) const;
-       int minimal_run (boost::function<bool (ContentTime, ContentTime, int)>);
+       int minimal_run (boost::function<bool (boost::optional<ContentTime>, boost::optional<ContentTime>, int)>);
        void seek_and_flush (int64_t);
 
        AVCodecContext* _subtitle_codec_context; ///< may be 0 if there is no subtitle
index daefd6db09a70d2f41d7c31138a384aece085352..17b44cdb8ca6f22fc0b2918d2f5627df58707b12 100644 (file)
@@ -142,7 +142,8 @@ Player::pass ()
        shared_ptr<DecodedSubtitle> ds = dynamic_pointer_cast<DecodedSubtitle> (earliest_decoded);
 
        if (dv && _video) {
-               if (!_just_did_inaccurate_seek && earliest_time > _video_position) {
+               DCPTime const frame = TIME_HZ / _film->video_frame_rate ();
+               if (!_just_did_inaccurate_seek && earliest_time > (_video_position + frame)) {
 
                        /* See if we're inside some video content */
                        list<shared_ptr<Piece> >::iterator i = _pieces.begin();
@@ -157,10 +158,19 @@ Player::pass ()
                                _last_incoming_video.video->dcp_time = _video_position;
                                emit_video (_last_incoming_video.weak_piece, _last_incoming_video.video);
                        }
+
                } else {
-                       emit_video (earliest_piece, dv);
+                       if (
+                               dv->dcp_time >= _video_position &&
+                               !earliest_piece->content->trimmed (dv->dcp_time - earliest_piece->content->position ())
+                               ) {
+                               
+                               emit_video (earliest_piece, dv);
+                       }
+               
                        earliest_piece->decoder->get ();
                }
+
        } else if (da && _audio) {
                if (!_just_did_inaccurate_seek && earliest_time > _audio_position) {
                        emit_silence (earliest_time - _audio_position);
@@ -196,16 +206,6 @@ Player::emit_video (weak_ptr<Piece> weak_piece, shared_ptr<DecodedVideo> video)
        assert (content);
 
        FrameRateChange frc (content->video_frame_rate(), _film->video_frame_rate());
-#if 0
-       XXX
-       if (frc.skip && (frame % 2) == 1) {
-               return;
-       }
-#endif 
-
-       if (content->trimmed (video->dcp_time - content->position ())) {
-               return;
-       }
 
        float const ratio = content->ratio() ? content->ratio()->ratio() : content->video_size_after_crop().ratio();
        libdcp::Size image_size = fit_ratio_within (ratio, _video_container_size);
index e66a1918cd1ec1aa00349af5c691124680e6f4a1..3c175f08c320c1268f1569c83380e10d67e4811c 100644 (file)
 
 */
 
+/** @file  test/ffmpeg_seek_test.cc
+ *  @brief Test seek using Player with an FFmpegDecoder; note that the player
+ *  can hide problems with FFmpegDecoder seeking as it will skip frames / insert
+ *  black as it sees fit.
+ */
+
 #include <boost/test/unit_test.hpp>
 #include "lib/player.h"
 #include "lib/ffmpeg_decoder.h"
@@ -88,10 +94,11 @@ check (shared_ptr<Player> p, DCPTime t)
        BOOST_CHECK ((first_audio.get() % (TIME_HZ / film->audio_frame_rate())) == 0);
 }
 
+/* Test basic seeking */
 BOOST_AUTO_TEST_CASE (ffmpeg_seek_test)
 {
-       film = new_test_film ("ffmpeg_audio_test");
-       film->set_name ("ffmpeg_audio_test");
+       film = new_test_film ("ffmpeg_seek_test");
+       film->set_name ("ffmpeg_seek_test");
        film->set_container (Ratio::from_id ("185"));
        shared_ptr<FFmpegContent> c (new FFmpegContent (film, "test/data/staircase.mov"));
        c->set_ratio (Ratio::from_id ("185"));
@@ -108,5 +115,3 @@ BOOST_AUTO_TEST_CASE (ffmpeg_seek_test)
        check (player, 0.2 * TIME_HZ);
        check (player, 0.3 * TIME_HZ);
 }
-
-
diff --git a/test/repeat_frame_test.cc b/test/repeat_frame_test.cc
new file mode 100644 (file)
index 0000000..52888e2
--- /dev/null
@@ -0,0 +1,51 @@
+/*
+    Copyright (C) 2013 Carl Hetherington <cth@carlh.net>
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+*/
+
+#include <boost/test/unit_test.hpp>
+#include "test.h"
+#include "lib/film.h"
+#include "lib/ratio.h"
+#include "lib/ffmpeg_content.h"
+#include "lib/dcp_content_type.h"
+
+using boost::shared_ptr;
+
+/* Test the repeat of frames by the player when putting a 24fps
+   source into a 48fps DCP.
+*/
+BOOST_AUTO_TEST_CASE (repeat_frame_test)
+{
+       shared_ptr<Film> film = new_test_film ("repeat_frame_test");
+       film->set_name ("repeat_frame_test");
+       film->set_container (Ratio::from_id ("185"));
+       film->set_dcp_content_type (DCPContentType::from_pretty_name ("Test"));
+       shared_ptr<FFmpegContent> c (new FFmpegContent (film, "test/data/red_24.mp4"));
+       c->set_ratio (Ratio::from_id ("185"));
+       film->examine_and_add_content (c);
+
+       wait_for_jobs ();
+
+       film->set_video_frame_rate (48);
+       film->make_dcp ();
+       wait_for_jobs ();
+
+       std::cout << film->dir (film->dcp_name ()) << "\n";
+       check_dcp ("test/data/repeat_frame_test", film->dir (film->dcp_name ()));
+}
+
diff --git a/test/seek_zero_test.cc b/test/seek_zero_test.cc
new file mode 100644 (file)
index 0000000..d298a77
--- /dev/null
@@ -0,0 +1,58 @@
+/*
+    Copyright (C) 2013 Carl Hetherington <cth@carlh.net>
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+*/
+
+/** @file  test/seek_zero_test.cc
+ *  @brief Test seek to zero with a raw FFmpegDecoder (without the player
+ *  confusing things as it might in ffmpeg_seek_test).
+ */
+
+#include <boost/test/unit_test.hpp>
+#include "lib/film.h"
+#include "lib/ffmpeg_content.h"
+#include "lib/ratio.h"
+#include "lib/dcp_content_type.h"
+#include "lib/ffmpeg_decoder.h"
+#include "test.h"
+
+using std::cout;
+using boost::shared_ptr;
+
+BOOST_AUTO_TEST_CASE (seek_zero_test)
+{
+       shared_ptr<Film> film = new_test_film ("seek_zero_test");
+       film->set_name ("seek_zero_test");
+       film->set_container (Ratio::from_id ("185"));
+       film->set_dcp_content_type (DCPContentType::from_pretty_name ("Test"));
+       shared_ptr<FFmpegContent> content (new FFmpegContent (film, "test/data/count300bd48.m2ts"));
+       content->set_ratio (Ratio::from_id ("185"));
+       film->examine_and_add_content (content);
+       wait_for_jobs ();
+
+       FFmpegDecoder decoder (film, content, true, false);
+       shared_ptr<Decoded> a = decoder.get ();
+       cout << a->content_time << "\n";
+       decoder.seek (0, true);
+       shared_ptr<Decoded> b = decoder.get ();
+       cout << b->content_time << "\n";
+
+       /* a will be after no seek, and b after a seek to zero, which should
+          have the same effect.
+       */
+       BOOST_CHECK_EQUAL (a->content_time, b->content_time);
+}
diff --git a/test/skip_frame_test.cc b/test/skip_frame_test.cc
new file mode 100644 (file)
index 0000000..4420475
--- /dev/null
@@ -0,0 +1,52 @@
+/*
+    Copyright (C) 2013 Carl Hetherington <cth@carlh.net>
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+*/
+
+#include <boost/test/unit_test.hpp>
+#include "test.h"
+#include "lib/film.h"
+#include "lib/ratio.h"
+#include "lib/ffmpeg_content.h"
+#include "lib/dcp_content_type.h"
+
+using boost::shared_ptr;
+
+/* Test the skip of frames by the player when putting a 48fps
+   source into a 24fps DCP.
+*/
+BOOST_AUTO_TEST_CASE (skip_frame_test)
+{
+       shared_ptr<Film> film = new_test_film ("skip_frame_test");
+       film->set_name ("skip_frame_test");
+       film->set_container (Ratio::from_id ("185"));
+       film->set_dcp_content_type (DCPContentType::from_pretty_name ("Test"));
+       shared_ptr<FFmpegContent> c (new FFmpegContent (film, "test/data/count300bd48.m2ts"));
+       c->set_ratio (Ratio::from_id ("185"));
+       film->examine_and_add_content (c);
+
+       wait_for_jobs ();
+       film->write_metadata ();
+
+       film->set_video_frame_rate (24);
+       film->make_dcp ();
+       wait_for_jobs ();
+
+       std::cout << film->dir (film->dcp_name ()) << "\n";
+       check_dcp ("test/data/skip_frame_test", film->dir (film->dcp_name ()));
+}
+
index 1a924ba0d0c5f8ea18fff197d5f9c70223c98291..3c7484fcdb48f17521bbf626fa8544785d8de2dd 100644 (file)
@@ -39,7 +39,9 @@ def build(bld):
                  repeat_frame_test.cc
                  resampler_test.cc
                  scaling_test.cc
+                 seek_zero_test.cc
                  silence_padding_test.cc
+                 skip_frame_test.cc
                  stream_test.cc
                  test.cc
                  threed_test.cc