diff --git a/doc/Changelog.md b/doc/Changelog.md index a9444a29..1fdb2a4d 100644 --- a/doc/Changelog.md +++ b/doc/Changelog.md @@ -3,6 +3,8 @@ Changelog {#changelog} # Release 1.5 (git master) +* [275](https://github.com/BlueBrain/Tide/pull/275): + Fix a crash that could occur when resizing large pixel streams. * [272](https://github.com/BlueBrain/Tide/pull/272): - Smooth transitions between resolution levels for opaque contents (PDFs, regular TIFF pyramids). diff --git a/tide/wall/datasources/PixelStreamUpdater.cpp b/tide/wall/datasources/PixelStreamUpdater.cpp index 47597496..19150b6a 100644 --- a/tide/wall/datasources/PixelStreamUpdater.cpp +++ b/tide/wall/datasources/PixelStreamUpdater.cpp @@ -91,24 +91,28 @@ const QString& PixelStreamUpdater::getUri() const ImagePtr PixelStreamUpdater::getTileImage(const uint tileIndex, const deflect::View view) const { - if (!_frameLeftOrMono) - { - print_log(LOG_ERROR, LOG_STREAM, "No frames yet"); - return ImagePtr(); - } - // guard against frame swap during asynchronous readings const QReadLocker frameLock(&_frameMutex); - const bool rightEye = view == deflect::View::right_eye; - const bool rightFrame = rightEye && !_frameRight->tiles.empty(); - const auto& processor = rightFrame ? _processRight : _processorLeft; - - // turbojpeg handles need to be per thread, and this function may be - // called from multiple threads - static QThreadStorage tileDecoders; try { + if (!_frameLeftOrMono) + throw std::runtime_error("No frames yet"); + + if (tileIndex >= _perTileLock->size()) + throw std::runtime_error("Tile index is invalid"); + + // prevent double-decoding of a tile that could occur unexpectedly when + // resizing the stream window + std::lock_guard lock{_perTileLock->at(tileIndex)}; + + const bool rightEye = view == deflect::View::right_eye; + const bool rightFrame = rightEye && !_frameRight->tiles.empty(); + const auto& processor = rightFrame ? _processRight : _processorLeft; + + // turbojpeg handles need to be per thread, and this function may be + // called from multiple threads + static QThreadStorage tileDecoders; return processor->getTileImage(tileIndex, tileDecoders.localData()); } catch (const std::runtime_error& e) @@ -140,10 +144,8 @@ Indices PixelStreamUpdater::computeVisibleSet(const QRectF& visibleTilesArea, const uint channel) const { Q_UNUSED(lod); - if (!_frameLeftOrMono || visibleTilesArea.isEmpty()) return Indices{}; - return _processorLeft->computeVisibleSet(visibleTilesArea, channel); } @@ -190,6 +192,7 @@ void PixelStreamUpdater::_onFrameSwapped(deflect::server::FramePtr frame) _frameLeftOrMono = std::move(leftOrMono); _frameRight = std::move(right); _createFrameProcessors(); + _createPerTileMutexes(); } emit pictureUpdated(); @@ -216,3 +219,12 @@ void PixelStreamUpdater::_createFrameProcessors() _processRight.reset(new PixelStreamPassthrough(_frameRight)); } } + +void PixelStreamUpdater::_createPerTileMutexes() +{ + const auto tiles = (_processorLeft ? _processorLeft->getTilesCount() : 0) + + (_processRight ? _processRight->getTilesCount() : 0); + + if (!_perTileLock || _perTileLock->size() < tiles) + _perTileLock = std::make_unique>(tiles); +} diff --git a/tide/wall/datasources/PixelStreamUpdater.h b/tide/wall/datasources/PixelStreamUpdater.h index ca08e964..225d6799 100644 --- a/tide/wall/datasources/PixelStreamUpdater.h +++ b/tide/wall/datasources/PixelStreamUpdater.h @@ -113,10 +113,12 @@ class PixelStreamUpdater : public QObject, public DataSource std::unique_ptr _processorLeft; std::unique_ptr _processRight; mutable QReadWriteLock _frameMutex; + mutable std::unique_ptr> _perTileLock; bool _readyToSwap = true; void _onFrameSwapped(deflect::server::FramePtr frame); void _createFrameProcessors(); + void _createPerTileMutexes(); }; #endif diff --git a/tide/wall/tools/PixelStreamAssembler.cpp b/tide/wall/tools/PixelStreamAssembler.cpp index 8f20e591..d53d922c 100644 --- a/tide/wall/tools/PixelStreamAssembler.cpp +++ b/tide/wall/tools/PixelStreamAssembler.cpp @@ -39,6 +39,8 @@ #include "PixelStreamAssembler.h" +#include // std::accumulate + namespace { Indices _mapToGlobalIndices(const Indices& perChannelIndices, @@ -58,7 +60,7 @@ PixelStreamAssembler::PixelStreamAssembler(deflect::server::FramePtr frame) } ImagePtr PixelStreamAssembler::getTileImage( - uint tileIndex, deflect::server::TileDecoder& decoder) + const uint tileIndex, deflect::server::TileDecoder& decoder) { const auto& channel = _getChannel(tileIndex); return channel.assembler.getTileImage(tileIndex - channel.offset, decoder); @@ -79,6 +81,14 @@ Indices PixelStreamAssembler::computeVisibleSet(const QRectF& visibleArea, return _mapToGlobalIndices(indices, channel.offset); } +size_t PixelStreamAssembler::getTilesCount() const +{ + return std::accumulate(_channels.begin(), _channels.end(), 0u, + [](const auto count, const Channel& channel) { + return count + channel.tilesCount; + }); +} + bool PixelStreamAssembler::_parseChannels(deflect::server::FramePtr frame) { if (frame->tiles.empty()) diff --git a/tide/wall/tools/PixelStreamAssembler.h b/tide/wall/tools/PixelStreamAssembler.h index 16ce6e91..135ca5c7 100644 --- a/tide/wall/tools/PixelStreamAssembler.h +++ b/tide/wall/tools/PixelStreamAssembler.h @@ -77,6 +77,9 @@ class PixelStreamAssembler : public PixelStreamProcessor Indices computeVisibleSet(const QRectF& visibleArea, uint channel) const final; + /** @copydoc PixelStreamProcessor::getTilesCount */ + size_t getTilesCount() const final; + private: struct Channel { diff --git a/tide/wall/tools/PixelStreamChannelAssembler.cpp b/tide/wall/tools/PixelStreamChannelAssembler.cpp index b3772f5e..6e2a0251 100644 --- a/tide/wall/tools/PixelStreamChannelAssembler.cpp +++ b/tide/wall/tools/PixelStreamChannelAssembler.cpp @@ -130,7 +130,7 @@ Indices PixelStreamChannelAssembler::computeVisibleSet( return visibleSet; } -uint PixelStreamChannelAssembler::getTilesCount() const +size_t PixelStreamChannelAssembler::getTilesCount() const { return _getTilesX() * _getTilesY(); } diff --git a/tide/wall/tools/PixelStreamChannelAssembler.h b/tide/wall/tools/PixelStreamChannelAssembler.h index f525379c..1ab2c6f2 100644 --- a/tide/wall/tools/PixelStreamChannelAssembler.h +++ b/tide/wall/tools/PixelStreamChannelAssembler.h @@ -78,8 +78,8 @@ class PixelStreamChannelAssembler : public PixelStreamProcessor Indices computeVisibleSet(const QRectF& visibleArea, uint channel) const final; - /** @return the number of tiles in the assembled image. */ - uint getTilesCount() const; + /** @copydoc PixelStreamProcessor::getTilesCount */ + size_t getTilesCount() const final; private: deflect::server::FramePtr _frame; diff --git a/tide/wall/tools/PixelStreamPassthrough.cpp b/tide/wall/tools/PixelStreamPassthrough.cpp index 3a4926ef..017e1700 100644 --- a/tide/wall/tools/PixelStreamPassthrough.cpp +++ b/tide/wall/tools/PixelStreamPassthrough.cpp @@ -45,7 +45,7 @@ #include PixelStreamPassthrough::PixelStreamPassthrough(deflect::server::FramePtr frame) - : _frame(frame) + : _frame{std::move(frame)} { } @@ -81,3 +81,8 @@ Indices PixelStreamPassthrough::computeVisibleSet(const QRectF& visibleArea, } return visibleSet; } + +size_t PixelStreamPassthrough::getTilesCount() const +{ + return _frame->tiles.size(); +} diff --git a/tide/wall/tools/PixelStreamPassthrough.h b/tide/wall/tools/PixelStreamPassthrough.h index e4b4f7bf..c3157c63 100644 --- a/tide/wall/tools/PixelStreamPassthrough.h +++ b/tide/wall/tools/PixelStreamPassthrough.h @@ -65,6 +65,9 @@ class PixelStreamPassthrough : public PixelStreamProcessor Indices computeVisibleSet(const QRectF& visibleArea, uint channel) const final; + /** @copydoc PixelStreamProcessor::getTilesCount */ + size_t getTilesCount() const final; + private: deflect::server::FramePtr _frame; }; diff --git a/tide/wall/tools/PixelStreamProcessor.h b/tide/wall/tools/PixelStreamProcessor.h index 2c7ec948..8d96e7e4 100644 --- a/tide/wall/tools/PixelStreamProcessor.h +++ b/tide/wall/tools/PixelStreamProcessor.h @@ -69,6 +69,9 @@ class PixelStreamProcessor virtual Indices computeVisibleSet(const QRectF& visibleArea, uint channel) const = 0; + /** @return the total number of assembled tiles. */ + virtual size_t getTilesCount() const = 0; + protected: /** @return the coordinates of the tile as a QRect. */ QRect toRect(const deflect::server::Tile& tile) const;