Ignore and report failures to decode frames during playback (#1593).

diff --git a/src/lib/image_proxy.h b/src/lib/image_proxy.h
index 9619fab75d345fe53b3ad278d4466756dc5dbcd0..1d57b4e08e27b746b38fdd2c4e9584ac4e66d4a8 100644 (file)
--- a/src/lib/image_proxy.h
+++ b/src/lib/image_proxy.h
@@ -64,6 +64,13 @@ public:
                Result (boost::shared_ptr<Image> image_, int log2_scaling_)
                        : image (image_)
                        , log2_scaling (log2_scaling_)
+                       , error (false)
+               {}
+
+               Result (boost::shared_ptr<Image> image_, int log2_scaling_, bool error_)
+                       : image (image_)
+                       , log2_scaling (log2_scaling_)
+                       , error (error_)
                {}
 
                /** Image (which will be aligned) */
@@ -73,6 +80,8 @@ public:
                 *  will be 1.
                 */
                int log2_scaling;
+               /** true if there was an error during image decoding, otherwise false */
+               bool error;
        };
 
        /** @param log Log to write to, or 0.

diff --git a/src/lib/j2k_image_proxy.cc b/src/lib/j2k_image_proxy.cc
index da3e23caf7bf64fd621fd74ee4cdfc8fe356b245..0e3fa88f5070182de40686dd06bf6629c458569d 100644 (file)
--- a/src/lib/j2k_image_proxy.cc
+++ b/src/lib/j2k_image_proxy.cc
@@ -51,6 +51,7 @@ J2KImageProxy::J2KImageProxy (boost::filesystem::path path, dcp::Size size, AVPi
        : _data (path)
        , _size (size)
        , _pixel_format (pixel_format)
+       , _error (false)
 {
        /* ::image assumes 16bpp */
        DCPOMATIC_ASSERT (_pixel_format == AV_PIX_FMT_RGB48 || _pixel_format == AV_PIX_FMT_XYZ12LE);
@@ -66,6 +67,7 @@ J2KImageProxy::J2KImageProxy (
        , _size (size)
        , _pixel_format (pixel_format)
        , _forced_reduction (forced_reduction)
+       , _error (false)
 {
        /* ::image assumes 16bpp */
        DCPOMATIC_ASSERT (_pixel_format == AV_PIX_FMT_RGB48 || _pixel_format == AV_PIX_FMT_XYZ12LE);
@@ -83,6 +85,7 @@ J2KImageProxy::J2KImageProxy (
        , _eye (eye)
        , _pixel_format (pixel_format)
        , _forced_reduction (forced_reduction)
+       , _error (false)
 {
        /* ::image assumes 16bpp */
        DCPOMATIC_ASSERT (_pixel_format == AV_PIX_FMT_RGB48 || _pixel_format == AV_PIX_FMT_XYZ12LE);
@@ -99,6 +102,7 @@ J2KImageProxy::J2KImageProxy (
 }
 
 J2KImageProxy::J2KImageProxy (shared_ptr<cxml::Node> xml, shared_ptr<Socket> socket)
+       : _error (false)
 {
        _size = dcp::Size (xml->number_child<int> ("Width"), xml->number_child<int> ("Height"));
        if (xml->optional_number_child<int> ("Eye")) {
@@ -136,29 +140,35 @@ J2KImageProxy::prepare (optional<dcp::Size> target_size) const
                reduce = max (0, reduce);
        }
 
-       shared_ptr<dcp::OpenJPEGImage> decompressed = dcp::decompress_j2k (const_cast<uint8_t*> (_data.data().get()), _data.size (), reduce);
-       _image.reset (new Image (_pixel_format, decompressed->size(), true));
-
-       int const shift = 16 - decompressed->precision (0);
-
-       /* Copy data in whatever format (sRGB or XYZ) into our Image; I'm assuming
-          the data is 12-bit either way.
-       */
-
-       int const width = decompressed->size().width;
-
-       int p = 0;
-       int* decomp_0 = decompressed->data (0);
-       int* decomp_1 = decompressed->data (1);
-       int* decomp_2 = decompressed->data (2);
-       for (int y = 0; y < decompressed->size().height; ++y) {
-               uint16_t* q = (uint16_t *) (_image->data()[0] + y * _image->stride()[0]);
-               for (int x = 0; x < width; ++x) {
-                       *q++ = decomp_0[p] << shift;
-                       *q++ = decomp_1[p] << shift;
-                       *q++ = decomp_2[p] << shift;
-                       ++p;
+       try {
+               shared_ptr<dcp::OpenJPEGImage> decompressed = dcp::decompress_j2k (const_cast<uint8_t*> (_data.data().get()), _data.size (), reduce);
+               _image.reset (new Image (_pixel_format, decompressed->size(), true));
+
+               int const shift = 16 - decompressed->precision (0);
+
+               /* Copy data in whatever format (sRGB or XYZ) into our Image; I'm assuming
+                  the data is 12-bit either way.
+                  */
+
+               int const width = decompressed->size().width;
+
+               int p = 0;
+               int* decomp_0 = decompressed->data (0);
+               int* decomp_1 = decompressed->data (1);
+               int* decomp_2 = decompressed->data (2);
+               for (int y = 0; y < decompressed->size().height; ++y) {
+                       uint16_t* q = (uint16_t *) (_image->data()[0] + y * _image->stride()[0]);
+                       for (int x = 0; x < width; ++x) {
+                               *q++ = decomp_0[p] << shift;
+                               *q++ = decomp_1[p] << shift;
+                               *q++ = decomp_2[p] << shift;
+                               ++p;
+                       }
                }
+       } catch (dcp::J2KDecompressionError& e) {
+               _image.reset (new Image (_pixel_format, _size, true));
+               _image->make_black ();
+               _error = true;
        }
 
        _target_size = target_size;
@@ -172,12 +182,14 @@ ImageProxy::Result
 J2KImageProxy::image (optional<dcp::Size> target_size) const
 {
        int const r = prepare (target_size);
+
        /* I think this is safe without a lock on mutex.  _image is guaranteed to be
           set up when prepare() has happened.
        */
-       return Result (_image, r);
+       return Result (_image, r, _error);
 }
 
+
 void
 J2KImageProxy::add_metadata (xmlpp::Node* node) const
 {

diff --git a/src/lib/j2k_image_proxy.h b/src/lib/j2k_image_proxy.h
index ec99e71a938c19c0b39d9af4c26d4a9baccf8f78..78f291e5d87426b7e910fb224167aee721627840 100644 (file)
--- a/src/lib/j2k_image_proxy.h
+++ b/src/lib/j2k_image_proxy.h
@@ -85,4 +85,6 @@ private:
        AVPixelFormat _pixel_format;
        mutable boost::mutex _mutex;
        boost::optional<int> _forced_reduction;
+       /** true if an error occurred while decoding the JPEG2000 data, false if not */
+       mutable bool _error;
 };

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 304f8c723d86daa5b6059b7496c84b4f978b59b2..d5a558184196100a6ebc94eb660e6b30e0b186f5 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -346,7 +346,8 @@ Player::black_player_video_frame (Eyes eyes) const
                        PresetColourConversion::all().front().conversion,
                        VIDEO_RANGE_FULL,
                        boost::weak_ptr<Content>(),
-                       boost::optional<Frame>()
+                       boost::optional<Frame>(),
+                       false
                )
        );
 }
@@ -850,7 +851,8 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
                        piece->content->video->colour_conversion(),
                        piece->content->video->range(),
                        piece->content,
-                       video.frame
+                       video.frame,
+                       false
                        )
                );
 

diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc
index bd643af60c4c248d777bb2e16b59c526c29b981b..8d55ffb2e2e5cf51db512f3bf90aacd1f64583e8 100644 (file)
--- a/src/lib/player_video.cc
+++ b/src/lib/player_video.cc
@@ -54,7 +54,8 @@ PlayerVideo::PlayerVideo (
        optional<ColourConversion> colour_conversion,
        VideoRange video_range,
        weak_ptr<Content> content,
-       optional<Frame> video_frame
+       optional<Frame> video_frame,
+       bool error
        )
        : _in (in)
        , _crop (crop)
@@ -67,6 +68,7 @@ PlayerVideo::PlayerVideo (
        , _video_range (video_range)
        , _content (content)
        , _video_frame (video_frame)
+       , _error (error)
 {
 
 }
@@ -81,6 +83,7 @@ PlayerVideo::PlayerVideo (shared_ptr<cxml::Node> node, shared_ptr<Socket> socket
        _eyes = (Eyes) node->number_child<int> ("Eyes");
        _part = (Part) node->number_child<int> ("Part");
        _video_range = (VideoRange) node->number_child<int>("VideoRange");
+       _error = node->optional_bool_child("Error").get_value_or (false);
 
        /* Assume that the ColourConversion uses the current state version */
        _colour_conversion = ColourConversion::from_xml (node, Film::current_state_version);
@@ -133,6 +136,7 @@ PlayerVideo::make_image (function<AVPixelFormat (AVPixelFormat)> pixel_format, b
        _image_fade = _fade;
 
        ImageProxy::Result prox = _in->image (_inter_size);
+       _error = prox.error;
 
        Crop total_crop = _crop;
        switch (_part) {
@@ -194,6 +198,7 @@ PlayerVideo::add_metadata (xmlpp::Node* node) const
        node->add_child("Eyes")->add_child_text (raw_convert<string> (static_cast<int> (_eyes)));
        node->add_child("Part")->add_child_text (raw_convert<string> (static_cast<int> (_part)));
        node->add_child("VideoRange")->add_child_text(raw_convert<string>(static_cast<int>(_video_range)));
+       node->add_child("Error")->add_child_text(_error ? "1" : "0");
        if (_colour_conversion) {
                _colour_conversion.get().as_xml (node);
        }
@@ -315,7 +320,8 @@ PlayerVideo::shallow_copy () const
                        _colour_conversion,
                        _video_range,
                        _content,
-                       _video_frame
+                       _video_frame,
+                       _error
                        )
                );
 }

diff --git a/src/lib/player_video.h b/src/lib/player_video.h
index 3cd5594097baa42e7a3727ba59c83d6ae0544d18..0a6a9da67858cff0e97d9fb30921b5f901422976 100644 (file)
--- a/src/lib/player_video.h
+++ b/src/lib/player_video.h
@@ -56,7 +56,8 @@ public:
                boost::optional<ColourConversion>,
                VideoRange video_range,
                boost::weak_ptr<Content>,
-               boost::optional<Frame>
+               boost::optional<Frame>,
+               bool error
                );
 
        PlayerVideo (boost::shared_ptr<cxml::Node>, boost::shared_ptr<Socket>);
@@ -107,6 +108,10 @@ public:
                return _content;
        }
 
+       bool error () const {
+               return _error;
+       }
+
 private:
        void make_image (boost::function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast) const;
 
@@ -137,6 +142,8 @@ private:
        mutable dcp::Size _image_out_size;
        /** _fade that was used to make _image */
        mutable boost::optional<double> _image_fade;
+       /** true if there was an error when decoding our image */
+       mutable bool _error;
 };
 
 #endif

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index 9c3a9c81e0ca8d5b11a10c4ac7cbda386b655c73..c020a3baf06d1222417673285bb360920952c8e6 100644 (file)
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -679,6 +679,14 @@ FilmViewer::dropped () const
        return _video_view->dropped ();
 }
 
+
+int
+FilmViewer::errored () const
+{
+       return _video_view->errored ();
+}
+
+
 int
 FilmViewer::gets () const
 {

diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h
index 60cde60d00139b10fb6a6546995e1eca66b50196..c54ff6eb44e78301689445ca4e4629afc4c76661 100644 (file)
--- a/src/wx/film_viewer.h
+++ b/src/wx/film_viewer.h
@@ -96,6 +96,7 @@ public:
        boost::optional<dcpomatic::DCPTime> audio_time () const;
 
        int dropped () const;
+       int errored () const;
        int gets () const;
 
        int audio_callback (void* out, unsigned int frames);

diff --git a/src/wx/player_information.cc b/src/wx/player_information.cc
index 5f480d5b4902832b14886a3aeb726c1d154fb7e8..9a569c00cc127226dd0d3c6c3db76cef9df1d812 100644 (file)
--- a/src/wx/player_information.cc
+++ b/src/wx/player_information.cc
@@ -90,7 +90,13 @@ PlayerInformation::periodic_update ()
 {
        shared_ptr<FilmViewer> fv = _viewer.lock ();
        if (fv) {
-               checked_set (_dropped, wxString::Format(_("Dropped frames: %d"), fv->dropped()));
+               wxString s = wxString::Format(_("Dropped frames: %d"), fv->dropped() + fv->errored());
+               if (fv->errored() == 1) {
+                       s += wxString::Format(_(" (%d error)"), fv->errored());
+               } else if (fv->errored() > 1) {
+                       s += wxString::Format(_(" (%d errors)"), fv->errored());
+               }
+               checked_set (_dropped, s);
        }
 }
 

diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc
index 014524757169cfad6140ee5b541f2a6ab0f46856..b9c45631edf1f6d583ac429e7473f8063288b773 100644 (file)
--- a/src/wx/video_view.cc
+++ b/src/wx/video_view.cc
@@ -38,6 +38,7 @@ VideoView::VideoView (FilmViewer* viewer)
        , _eyes (EYES_LEFT)
        , _three_d (false)
        , _dropped (0)
+       , _errored (0)
        , _gets (0)
 {
 
@@ -83,6 +84,10 @@ VideoView::get_next_frame (bool non_blocking)
                _player_video.first->eyes() != EYES_BOTH
                );
 
+       if (_player_video.first && _player_video.first->error()) {
+               ++_errored;
+       }
+
        return true;
 }
 
@@ -114,6 +119,7 @@ VideoView::start ()
 {
        boost::mutex::scoped_lock lm (_mutex);
        _dropped = 0;
+       _errored = 0;
 }
 
 bool

diff --git a/src/wx/video_view.h b/src/wx/video_view.h
index f9e0670435ed13edaa2e919cc373e11d12a0a48d..50ea40fc76e081ae7ba16711d3a1c2adfdf9b66f 100644 (file)
--- a/src/wx/video_view.h
+++ b/src/wx/video_view.h
@@ -66,6 +66,11 @@ public:
                return _dropped;
        }
 
+       int errored () const {
+               boost::mutex::scoped_lock lm (_mutex);
+               return _errored;
+       }
+
        int gets () const {
                boost::mutex::scoped_lock lm (_mutex);
                return _gets;
@@ -157,6 +162,7 @@ private:
        bool _three_d;
 
        int _dropped;
+       int _errored;
        int _gets;
 };
 

diff --git a/test/client_server_test.cc b/test/client_server_test.cc
index df854f9f3efec8bbb364e0ecbec2aedcb2fe7037..a64f4d295e6686d4da7f7b40c28b09db2905cb06 100644 (file)
--- a/test/client_server_test.cc
+++ b/test/client_server_test.cc
@@ -99,7 +99,8 @@ BOOST_AUTO_TEST_CASE (client_server_test_rgb)
                        ColourConversion(),
                        VIDEO_RANGE_FULL,
                        weak_ptr<Content>(),
-                       optional<Frame>()
+                       optional<Frame>(),
+                       false
                        )
                );
 
@@ -184,7 +185,8 @@ BOOST_AUTO_TEST_CASE (client_server_test_yuv)
                        ColourConversion(),
                        VIDEO_RANGE_FULL,
                        weak_ptr<Content>(),
-                       optional<Frame>()
+                       optional<Frame>(),
+                       false
                        )
                );
 
@@ -256,7 +258,8 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k)
                        ColourConversion(),
                        VIDEO_RANGE_FULL,
                        weak_ptr<Content>(),
-                       optional<Frame>()
+                       optional<Frame>(),
+                       false
                        )
                );
 
@@ -284,7 +287,8 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k)
                        PresetColourConversion::all().front().conversion,
                        VIDEO_RANGE_FULL,
                        weak_ptr<Content>(),
-                       optional<Frame>()
+                       optional<Frame>(),
+                       false
                        )
                );