From 4ab86ef0295bcd6bb9297996a06006f371d22bae Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 14 Apr 2020 22:16:27 +0200 Subject: [PATCH] Ignore and report failures to decode frames during playback (#1593). --- src/lib/image_proxy.h | 9 ++++++ src/lib/j2k_image_proxy.cc | 58 ++++++++++++++++++++++-------------- src/lib/j2k_image_proxy.h | 2 ++ src/lib/player.cc | 6 ++-- src/lib/player_video.cc | 10 +++++-- src/lib/player_video.h | 9 +++++- src/wx/film_viewer.cc | 8 +++++ src/wx/film_viewer.h | 1 + src/wx/player_information.cc | 8 ++++- src/wx/video_view.cc | 6 ++++ src/wx/video_view.h | 6 ++++ test/client_server_test.cc | 12 +++++--- 12 files changed, 102 insertions(+), 33 deletions(-) diff --git a/src/lib/image_proxy.h b/src/lib/image_proxy.h index 9619fab75..1d57b4e08 100644 --- a/src/lib/image_proxy.h +++ b/src/lib/image_proxy.h @@ -64,6 +64,13 @@ public: Result (boost::shared_ptr image_, int log2_scaling_) : image (image_) , log2_scaling (log2_scaling_) + , error (false) + {} + + Result (boost::shared_ptr 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 da3e23caf..0e3fa88f5 100644 --- 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 xml, shared_ptr socket) + : _error (false) { _size = dcp::Size (xml->number_child ("Width"), xml->number_child ("Height")); if (xml->optional_number_child ("Eye")) { @@ -136,29 +140,35 @@ J2KImageProxy::prepare (optional target_size) const reduce = max (0, reduce); } - shared_ptr decompressed = dcp::decompress_j2k (const_cast (_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 decompressed = dcp::decompress_j2k (const_cast (_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 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 ec99e71a9..78f291e5d 100644 --- 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 _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 304f8c723..d5a558184 100644 --- 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(), - boost::optional() + boost::optional(), + false ) ); } @@ -850,7 +851,8 @@ Player::video (weak_ptr 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 bd643af60..8d55ffb2e 100644 --- a/src/lib/player_video.cc +++ b/src/lib/player_video.cc @@ -54,7 +54,8 @@ PlayerVideo::PlayerVideo ( optional colour_conversion, VideoRange video_range, weak_ptr content, - optional video_frame + optional 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 node, shared_ptr socket _eyes = (Eyes) node->number_child ("Eyes"); _part = (Part) node->number_child ("Part"); _video_range = (VideoRange) node->number_child("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 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 (static_cast (_eyes))); node->add_child("Part")->add_child_text (raw_convert (static_cast (_part))); node->add_child("VideoRange")->add_child_text(raw_convert(static_cast(_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 3cd559409..0a6a9da67 100644 --- a/src/lib/player_video.h +++ b/src/lib/player_video.h @@ -56,7 +56,8 @@ public: boost::optional, VideoRange video_range, boost::weak_ptr, - boost::optional + boost::optional, + bool error ); PlayerVideo (boost::shared_ptr, boost::shared_ptr); @@ -107,6 +108,10 @@ public: return _content; } + bool error () const { + return _error; + } + private: void make_image (boost::function 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 _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 9c3a9c81e..c020a3baf 100644 --- 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 60cde60d0..c54ff6eb4 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -96,6 +96,7 @@ public: boost::optional 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 5f480d5b4..9a569c00c 100644 --- a/src/wx/player_information.cc +++ b/src/wx/player_information.cc @@ -90,7 +90,13 @@ PlayerInformation::periodic_update () { shared_ptr 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 014524757..b9c45631e 100644 --- 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 f9e067043..50ea40fc7 100644 --- 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 df854f9f3..a64f4d295 100644 --- 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(), - optional() + optional(), + false ) ); @@ -184,7 +185,8 @@ BOOST_AUTO_TEST_CASE (client_server_test_yuv) ColourConversion(), VIDEO_RANGE_FULL, weak_ptr(), - optional() + optional(), + false ) ); @@ -256,7 +258,8 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k) ColourConversion(), VIDEO_RANGE_FULL, weak_ptr(), - optional() + optional(), + false ) ); @@ -284,7 +287,8 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k) PresetColourConversion::all().front().conversion, VIDEO_RANGE_FULL, weak_ptr(), - optional() + optional(), + false ) ); -- 2.30.2