From 466216662900d6362f8fa60ed67b778f60413968 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 24 Aug 2014 21:10:13 +0100 Subject: [PATCH] Fix tests. --- doc/design/player_get_audio.svg | 399 ++++++++++++++++++ doc/design/timing.svg | 645 ++++++++++++++++++++++++++++++ doc/design/who_fills_the_gaps.tex | 30 ++ test/player_test.cc | 2 +- test/test.cc | 49 ++- test/test.h | 1 + test/upmixer_a_test.cc | 12 +- 7 files changed, 1130 insertions(+), 8 deletions(-) create mode 100644 doc/design/player_get_audio.svg create mode 100644 doc/design/timing.svg create mode 100644 doc/design/who_fills_the_gaps.tex diff --git a/doc/design/player_get_audio.svg b/doc/design/player_get_audio.svg new file mode 100644 index 000000000..fe7bdd5a3 --- /dev/null +++ b/doc/design/player_get_audio.svg @@ -0,0 +1,399 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + image/svg+xml + + + + + + + + + DCP time 0 + + + + time + + length + + + length_frames + out + + + dcp_to_content_audio(time) + Content + + in + + in->frame + + diff --git a/doc/design/timing.svg b/doc/design/timing.svg new file mode 100644 index 000000000..30325e7db --- /dev/null +++ b/doc/design/timing.svg @@ -0,0 +1,645 @@ + + + + + + + + + + + + + + + + + + + + + + + image/svg+xml + + + + + + + FFmpeg sources are by far the most complicated, so we consider those only.Hardest video case:Content frames arrive with ContentTime; this is converted to DCP time and checked for validity against_video_position. _video_position is incremented by one DCP frame period each time a frame is emitted.Emission is timestamped with _video_position.In general, the Decoded::dcp_time is used as a check against the actual position in the DCP (basedon what has been emitted).Hardest audio case:Timestamps are not sample-accurate, here B should be after A but its timestamp says different. Thingsare further complicated by resampling, which renders timestamps invalid. The upshot is that audiotimestamps are basically useless. However, since audio is (apparently) always continuous in content fileswe can make our own timestamps. + + + + + + A + + + + B + + + + C + + + + D + + + + + + BLACK + A + D + B + C + C + BLACK + (outside content, so black) + + (repeat) + + + CONTENT + DCP(at higher frame rate) + + + + A + + + + B + + + + C + + + + D + + + diff --git a/doc/design/who_fills_the_gaps.tex b/doc/design/who_fills_the_gaps.tex new file mode 100644 index 000000000..00e8ac39f --- /dev/null +++ b/doc/design/who_fills_the_gaps.tex @@ -0,0 +1,30 @@ +\documentclass{article} +\begin{document} + +There is a lot of dancing about to handle potential gaps and sync +problems in the FFmpeg decoder. It might be nicer if +\texttt{FFmpegDecoder} could just spit out video and audio with +timestamps and let the player sort it out, since the player must +already handle insertion of black and silence. + +The first question would be what time unit the decoder should use to +stamp its output. Initially we have the PTS, in some time base, and +we can convert that to seconds at the content's frame rate; this is +basically a \texttt{Time}. So we could emit video and audio content +with \texttt{Time} stamps. + +Then the player receives video frames, and can fill in gaps. + +The FFmpeg decoder would still have to account for non-zero initial +PTS, as it is expected that such `dead time' is trimmed from the +source implicitly. + +The snag with this is that hitherto \texttt{Time} has meant DCP time, +not time at a content's rates (before the content is potentially sped +up). As it stands, seek takes a \texttt{Time} in the DCP and the +content class converts it to content frames. This is then (rather +grottily) converted back to time again via the content frame rate. +All a bit grim. Everything should probably work in time rather than +frame rates. + +\end{document} diff --git a/test/player_test.cc b/test/player_test.cc index d48060242..b6f864f82 100644 --- a/test/player_test.cc +++ b/test/player_test.cc @@ -49,7 +49,7 @@ BOOST_AUTO_TEST_CASE (player_overlaps_test) film->examine_and_add_content (C); wait_for_jobs (); - BOOST_CHECK_EQUAL (A->full_length(), DCPTime (280000)); + BOOST_CHECK_EQUAL (A->full_length(), DCPTime (288000)); A->set_position (DCPTime::from_seconds (0)); B->set_position (DCPTime::from_seconds (10)); diff --git a/test/test.cc b/test/test.cc index cbf4b5d4d..0211c2cb8 100644 --- a/test/test.cc +++ b/test/test.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include "lib/config.h" @@ -108,6 +109,45 @@ new_test_film (string name) return f; } +void +check_audio_file (boost::filesystem::path ref, boost::filesystem::path check) +{ + SF_INFO ref_info; + ref_info.format = 0; + SNDFILE* ref_file = sf_open (ref.string().c_str(), SFM_READ, &ref_info); + BOOST_CHECK (ref_file); + + SF_INFO check_info; + check_info.format = 0; + SNDFILE* check_file = sf_open (check.string().c_str(), SFM_READ, &check_info); + BOOST_CHECK (check_file); + + BOOST_CHECK_EQUAL (ref_info.frames, check_info.frames); + BOOST_CHECK_EQUAL (ref_info.samplerate, check_info.samplerate); + BOOST_CHECK_EQUAL (ref_info.channels, check_info.channels); + BOOST_CHECK_EQUAL (ref_info.format, check_info.format); + + /* buffer_size is in frames */ + sf_count_t const buffer_size = 65536 * ref_info.channels; + int32_t* ref_buffer = new int32_t[buffer_size]; + int32_t* check_buffer = new int32_t[buffer_size]; + + sf_count_t N = ref_info.frames; + while (N) { + sf_count_t this_time = min (buffer_size, N); + sf_count_t r = sf_readf_int (ref_file, ref_buffer, this_time); + BOOST_CHECK_EQUAL (r, this_time); + r = sf_readf_int (check_file, check_buffer, this_time); + BOOST_CHECK_EQUAL (r, this_time); + + for (sf_count_t i = 0; i < this_time; ++i) { + BOOST_CHECK (fabs (ref_buffer[i] - check_buffer[i]) <= 65536); + } + + N -= this_time; + } +} + void check_file (boost::filesystem::path ref, boost::filesystem::path check) { @@ -122,6 +162,9 @@ check_file (boost::filesystem::path ref, boost::filesystem::path check) uint8_t* ref_buffer = new uint8_t[buffer_size]; uint8_t* check_buffer = new uint8_t[buffer_size]; + SafeStringStream error; + error << "File " << check.string() << " differs from reference " << ref.string(); + while (N) { uintmax_t this_time = min (uintmax_t (buffer_size), N); size_t r = fread (ref_buffer, 1, this_time, ref_file); @@ -129,7 +172,11 @@ check_file (boost::filesystem::path ref, boost::filesystem::path check) r = fread (check_buffer, 1, this_time, check_file); BOOST_CHECK_EQUAL (r, this_time); - BOOST_CHECK_EQUAL (memcmp (ref_buffer, check_buffer, this_time), 0); + BOOST_CHECK_MESSAGE (memcmp (ref_buffer, check_buffer, this_time) == 0, error.str ()); + if (memcmp (ref_buffer, check_buffer, this_time)) { + break; + } + N -= this_time; } diff --git a/test/test.h b/test/test.h index 57ea32c57..c9e477e02 100644 --- a/test/test.h +++ b/test/test.h @@ -28,6 +28,7 @@ extern void wait_for_jobs (); extern boost::shared_ptr new_test_film (std::string); extern void check_dcp (boost::filesystem::path, boost::filesystem::path); extern void check_file (boost::filesystem::path ref, boost::filesystem::path check); +extern void check_audio_file (boost::filesystem::path ref, boost::filesystem::path check); extern void check_xml (boost::filesystem::path, boost::filesystem::path, std::list); extern void check_file (boost::filesystem::path, boost::filesystem::path); extern boost::filesystem::path test_film_dir (std::string); diff --git a/test/upmixer_a_test.cc b/test/upmixer_a_test.cc index 67e909ed2..b115db21b 100644 --- a/test/upmixer_a_test.cc +++ b/test/upmixer_a_test.cc @@ -71,10 +71,10 @@ BOOST_AUTO_TEST_CASE (upmixer_a_test) sf_close (Ls); sf_close (Rs); - check_file ("test/data/upmixer_a_test/L.wav", "build/test/upmixer_a_test/L.wav"); - check_file ("test/data/upmixer_a_test/R.wav", "build/test/upmixer_a_test/R.wav"); - check_file ("test/data/upmixer_a_test/C.wav", "build/test/upmixer_a_test/C.wav"); - check_file ("test/data/upmixer_a_test/Lfe.wav", "build/test/upmixer_a_test/Lfe.wav"); - check_file ("test/data/upmixer_a_test/Ls.wav", "build/test/upmixer_a_test/Ls.wav"); - check_file ("test/data/upmixer_a_test/Rs.wav", "build/test/upmixer_a_test/Rs.wav"); + check_audio_file ("test/data/upmixer_a_test/L.wav", "build/test/upmixer_a_test/L.wav"); + check_audio_file ("test/data/upmixer_a_test/R.wav", "build/test/upmixer_a_test/R.wav"); + check_audio_file ("test/data/upmixer_a_test/C.wav", "build/test/upmixer_a_test/C.wav"); + check_audio_file ("test/data/upmixer_a_test/Lfe.wav", "build/test/upmixer_a_test/Lfe.wav"); + check_audio_file ("test/data/upmixer_a_test/Ls.wav", "build/test/upmixer_a_test/Ls.wav"); + check_audio_file ("test/data/upmixer_a_test/Rs.wav", "build/test/upmixer_a_test/Rs.wav"); } -- 2.30.2