From: Carl Hetherington Date: Wed, 19 Feb 2020 16:23:21 +0000 (+0100) Subject: Nicer fix for 2D-labelled-3D checking from master. X-Git-Tag: v2.15.43~5^2 X-Git-Url: https://main.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=1679c3dc40262733f46dda9f4151367bf93f2b76 Nicer fix for 2D-labelled-3D checking from master. --- diff --git a/src/lib/dcp_decoder.cc b/src/lib/dcp_decoder.cc index 03ac66e94..19831185d 100644 --- a/src/lib/dcp_decoder.cc +++ b/src/lib/dcp_decoder.cc @@ -29,6 +29,7 @@ #include "image.h" #include "config.h" #include "digester.h" +#include "frame_interval_checker.h" #include #include #include diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc index 2f92b9b7c..77b608fa8 100644 --- a/src/lib/ffmpeg_decoder.cc +++ b/src/lib/ffmpeg_decoder.cc @@ -42,6 +42,7 @@ #include "compose.hpp" #include "text_content.h" #include "audio_content.h" +#include "frame_interval_checker.h" #include #include #include diff --git a/src/lib/frame_interval_checker.cc b/src/lib/frame_interval_checker.cc new file mode 100644 index 000000000..27c0224ec --- /dev/null +++ b/src/lib/frame_interval_checker.cc @@ -0,0 +1,66 @@ +/* + Copyright (C) 2020 Carl Hetherington + + This file is part of DCP-o-matic. + + DCP-o-matic 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. + + DCP-o-matic 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 DCP-o-matic. If not, see . + +*/ + +#include "frame_interval_checker.h" + +using namespace dcpomatic; + +int const FrameIntervalChecker::_frames = 16; + +void +FrameIntervalChecker::feed (ContentTime time, double frame_rate) +{ + /* The caller isn't meant to feed too much data before + calling guess() and destroying the FrameIntervalChecker. + */ + DCPOMATIC_ASSERT (_intervals.size() < _frames); + + if (_last) { + _intervals.push_back ((time - *_last).seconds() * frame_rate); + } + + _last = time; +} + +FrameIntervalChecker::Guess +FrameIntervalChecker::guess () const +{ + if (_intervals.size() < _frames) { + /* How soon can you land? + * I can't tell. + * You can tell me, I'm a doctor. + * Nom I mean I'm just not sure. + * Can't you take a guess? + * Well, not for another two hours. + * You can't take a guess for another two hours? + */ + return AGAIN; + } + + int near_1 = 0; + BOOST_FOREACH (double i, _intervals) { + if (i > 0.5) { + ++near_1; + } + } + + return (near_1 < 3 * _frames / 4) ? PROBABLY_3D : PROBABLY_NOT_3D; +} + diff --git a/src/lib/frame_interval_checker.h b/src/lib/frame_interval_checker.h new file mode 100644 index 000000000..a8db31a49 --- /dev/null +++ b/src/lib/frame_interval_checker.h @@ -0,0 +1,50 @@ +/* + Copyright (C) 2020 Carl Hetherington + + This file is part of DCP-o-matic. + + DCP-o-matic 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. + + DCP-o-matic 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 DCP-o-matic. If not, see . + +*/ + +#include "dcpomatic_time.h" +#include +#include +#include + +class FrameIntervalChecker : public boost::noncopyable +{ +public: + void feed (dcpomatic::ContentTime time, double frame_rate); + + enum Guess + { + AGAIN, + PROBABLY_3D, + PROBABLY_NOT_3D + }; + + Guess guess () const; + +private: + boost::optional _last; + /** Intervals of last frames, in fractions of a frame; i.e. 1 in here + * means the last two frames were one frame interval apart according + * to the frame rate passed to ::feed(). + */ + std::vector _intervals; + + static int const _frames; +}; + diff --git a/src/lib/image_decoder.cc b/src/lib/image_decoder.cc index 8aa3f6a99..15187b11b 100644 --- a/src/lib/image_decoder.cc +++ b/src/lib/image_decoder.cc @@ -27,6 +27,7 @@ #include "film.h" #include "exceptions.h" #include "video_content.h" +#include "frame_interval_checker.h" #include #include diff --git a/src/lib/video_decoder.cc b/src/lib/video_decoder.cc index 508ed90b7..2850b5aa0 100644 --- a/src/lib/video_decoder.cc +++ b/src/lib/video_decoder.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2012-2018 Carl Hetherington + Copyright (C) 2012-2020 Carl Hetherington This file is part of DCP-o-matic. @@ -23,6 +23,7 @@ #include "raw_image_proxy.h" #include "film.h" #include "log.h" +#include "frame_interval_checker.h" #include "compose.hpp" #include #include @@ -40,6 +41,7 @@ using namespace dcpomatic; VideoDecoder::VideoDecoder (Decoder* parent, shared_ptr c) : DecoderPart (parent) , _content (c) + , _frame_interval_checker (new FrameIntervalChecker()) { } @@ -60,33 +62,33 @@ VideoDecoder::emit (shared_ptr film, shared_ptr im return; } - /* Before we `re-write' the frame indexes of these incoming data we need to check for - the case where the user has some 2D content which they have marked as 3D. With 3D - we should get two frames for each frame index, but in this `bad' case we only get - one. We need to throw an exception if this happens. - */ - - if (_content->video->frame_type() == VIDEO_FRAME_TYPE_3D) { - if (_last_threed_frames.size() > 4) { - _last_threed_frames.erase (_last_threed_frames.begin()); - } - _last_threed_frames.push_back (decoder_frame); - if (_last_threed_frames.size() == 4) { - if (_last_threed_frames[0] != _last_threed_frames[1] || _last_threed_frames[2] != _last_threed_frames[3]) { - boost::throw_exception ( - DecodeError( - String::compose( - _("The content file %1 is set as 3D but does not appear to contain 3D images. Please set it to 2D. " - "You can still make a 3D DCP from this content by ticking the 3D option in the DCP video tab."), - _content->path(0) - ) + double const afr = _content->active_video_frame_rate(film); + VideoFrameType const vft = _content->video->frame_type(); + + ContentTime frame_time = ContentTime::from_frames (decoder_frame, afr); + + /* Do some heuristics to try and spot the case where the user sets content to 3D + * when it is not. We try to tell this by looking at the differences in time between + * the first few frames. Real 3D content should have two frames for each timestamp. + */ + if (_frame_interval_checker) { + _frame_interval_checker->feed (frame_time, afr); + if (_frame_interval_checker->guess() == FrameIntervalChecker::PROBABLY_NOT_3D && vft == VIDEO_FRAME_TYPE_3D) { + boost::throw_exception ( + DecodeError( + String::compose( + _("The content file %1 is set as 3D but does not appear to contain 3D images. Please set it to 2D. " + "You can still make a 3D DCP from this content by ticking the 3D option in the DCP video tab."), + _content->path(0) ) - ); - } + ) + ); } - } - double const afr = _content->active_video_frame_rate(film); + if (_frame_interval_checker->guess() != FrameIntervalChecker::AGAIN) { + _frame_interval_checker.reset (); + } + } Frame frame; Eyes eyes = EYES_BOTH; @@ -100,15 +102,14 @@ VideoDecoder::emit (shared_ptr film, shared_ptr im If we drop the frame with the duplicated timestamp we obviously lose sync. */ _position = ContentTime::from_frames (decoder_frame, afr); - if (_content->video->frame_type() == VIDEO_FRAME_TYPE_3D_ALTERNATE) { + if (vft == VIDEO_FRAME_TYPE_3D_ALTERNATE) { frame = decoder_frame / 2; _last_emitted_eyes = EYES_RIGHT; } else { frame = decoder_frame; } } else { - VideoFrameType const ft = _content->video->frame_type (); - if (ft == VIDEO_FRAME_TYPE_3D_ALTERNATE || ft == VIDEO_FRAME_TYPE_3D) { + if (vft == VIDEO_FRAME_TYPE_3D || vft == VIDEO_FRAME_TYPE_3D_ALTERNATE) { DCPOMATIC_ASSERT (_last_emitted_eyes); if (_last_emitted_eyes.get() == EYES_RIGHT) { frame = _position->frames_round(afr) + 1; @@ -122,7 +123,7 @@ VideoDecoder::emit (shared_ptr film, shared_ptr im } } - switch (_content->video->frame_type ()) { + switch (vft) { case VIDEO_FRAME_TYPE_2D: Data (ContentVideo (image, frame, EYES_BOTH, PART_WHOLE)); break; @@ -163,7 +164,8 @@ VideoDecoder::emit (shared_ptr film, shared_ptr im void VideoDecoder::seek () { - _position = boost::optional(); + _position = boost::none; _last_emitted_frame.reset (); _last_emitted_eyes.reset (); + _frame_interval_checker.reset (new FrameIntervalChecker()); } diff --git a/src/lib/video_decoder.h b/src/lib/video_decoder.h index c8530d82c..dca8eef11 100644 --- a/src/lib/video_decoder.h +++ b/src/lib/video_decoder.h @@ -37,6 +37,7 @@ class VideoContent; class ImageProxy; class Image; class Log; +class FrameIntervalChecker; /** @class VideoDecoder * @brief Parent for classes which decode video. @@ -66,7 +67,7 @@ private: boost::optional _last_emitted_frame; boost::optional _last_emitted_eyes; boost::optional _position; - std::vector _last_threed_frames; + boost::scoped_ptr _frame_interval_checker; }; #endif diff --git a/src/lib/video_mxf_decoder.cc b/src/lib/video_mxf_decoder.cc index 1bf2b1bef..4482606f0 100644 --- a/src/lib/video_mxf_decoder.cc +++ b/src/lib/video_mxf_decoder.cc @@ -22,6 +22,7 @@ #include "video_decoder.h" #include "video_mxf_content.h" #include "j2k_image_proxy.h" +#include "frame_interval_checker.h" #include #include #include diff --git a/src/lib/wscript b/src/lib/wscript index b78586843..31071bf7a 100644 --- a/src/lib/wscript +++ b/src/lib/wscript @@ -107,6 +107,7 @@ sources = """ filter.cc ffmpeg_image_proxy.cc font.cc + frame_interval_checker.cc frame_rate_change.cc hints.cc internet.cc diff --git a/test/frame_interval_checker_test.cc b/test/frame_interval_checker_test.cc new file mode 100644 index 000000000..4b92d33d6 --- /dev/null +++ b/test/frame_interval_checker_test.cc @@ -0,0 +1,140 @@ +/* + Copyright (C) 2020 Carl Hetherington + + This file is part of DCP-o-matic. + + DCP-o-matic 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. + + DCP-o-matic 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 DCP-o-matic. If not, see . + +*/ + +#include "lib/frame_interval_checker.h" +#include + +using namespace dcpomatic; + +/** Test of 2D-ish frame timings */ +BOOST_AUTO_TEST_CASE (frame_interval_checker_test1) +{ + FrameIntervalChecker checker; + ContentTime t(3888); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4012); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4000); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4000); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(3776); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(3779); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4010); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4085); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4085); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4012); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4000); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4000); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(3776); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(3779); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4010); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4085); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4085); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::PROBABLY_NOT_3D); +} + +/** Test of 3D-ish frame timings */ +BOOST_AUTO_TEST_CASE (frame_interval_checker_test2) +{ + FrameIntervalChecker checker; + ContentTime t(3888); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(0); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4000); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(0); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(3776); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(50); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4010); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(2); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4011); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(0); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4000); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(0); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(3776); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(50); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4010); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(2); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::AGAIN); + t += ContentTime(4011); + checker.feed (t, 24); + BOOST_CHECK (checker.guess() == FrameIntervalChecker::PROBABLY_3D); +} + + diff --git a/test/wscript b/test/wscript index 3c6170dcf..0e777c803 100644 --- a/test/wscript +++ b/test/wscript @@ -79,6 +79,7 @@ def build(bld): file_log_test.cc file_naming_test.cc film_metadata_test.cc + frame_interval_checker_test.cc frame_rate_test.cc image_content_fade_test.cc image_filename_sorter_test.cc