Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix segfault affecting resizing of large streams with small tiles #275

Merged
merged 1 commit into from
Sep 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
42 changes: 27 additions & 15 deletions tide/wall/datasources/PixelStreamUpdater.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<deflect::server::TileDecoder> 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<std::mutex> 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<deflect::server::TileDecoder> tileDecoders;
return processor->getTileImage(tileIndex, tileDecoders.localData());
}
catch (const std::runtime_error& e)
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -190,6 +192,7 @@ void PixelStreamUpdater::_onFrameSwapped(deflect::server::FramePtr frame)
_frameLeftOrMono = std::move(leftOrMono);
_frameRight = std::move(right);
_createFrameProcessors();
_createPerTileMutexes();
}

emit pictureUpdated();
Expand All @@ -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<std::vector<std::mutex>>(tiles);
}
2 changes: 2 additions & 0 deletions tide/wall/datasources/PixelStreamUpdater.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,12 @@ class PixelStreamUpdater : public QObject, public DataSource
std::unique_ptr<PixelStreamProcessor> _processorLeft;
std::unique_ptr<PixelStreamProcessor> _processRight;
mutable QReadWriteLock _frameMutex;
mutable std::unique_ptr<std::vector<std::mutex>> _perTileLock;
bool _readyToSwap = true;

void _onFrameSwapped(deflect::server::FramePtr frame);
void _createFrameProcessors();
void _createPerTileMutexes();
};

#endif
12 changes: 11 additions & 1 deletion tide/wall/tools/PixelStreamAssembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@

#include "PixelStreamAssembler.h"

#include <numeric> // std::accumulate

namespace
{
Indices _mapToGlobalIndices(const Indices& perChannelIndices,
Expand All @@ -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);
Expand All @@ -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())
Expand Down
3 changes: 3 additions & 0 deletions tide/wall/tools/PixelStreamAssembler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
2 changes: 1 addition & 1 deletion tide/wall/tools/PixelStreamChannelAssembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ Indices PixelStreamChannelAssembler::computeVisibleSet(
return visibleSet;
}

uint PixelStreamChannelAssembler::getTilesCount() const
size_t PixelStreamChannelAssembler::getTilesCount() const
{
return _getTilesX() * _getTilesY();
}
Expand Down
4 changes: 2 additions & 2 deletions tide/wall/tools/PixelStreamChannelAssembler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion tide/wall/tools/PixelStreamPassthrough.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
#include <deflect/server/TileDecoder.h>

PixelStreamPassthrough::PixelStreamPassthrough(deflect::server::FramePtr frame)
: _frame(frame)
: _frame{std::move(frame)}
{
}

Expand Down Expand Up @@ -81,3 +81,8 @@ Indices PixelStreamPassthrough::computeVisibleSet(const QRectF& visibleArea,
}
return visibleSet;
}

size_t PixelStreamPassthrough::getTilesCount() const
{
return _frame->tiles.size();
}
3 changes: 3 additions & 0 deletions tide/wall/tools/PixelStreamPassthrough.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
3 changes: 3 additions & 0 deletions tide/wall/tools/PixelStreamProcessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down