Skip to content

Commit

Permalink
Fix possible segfaults with side-by-side 3D movies
Browse files Browse the repository at this point in the history
It was possible that the left & right PicturePtr& were reset
concurrently by _triggerFrameUpdate() while a getTileImage() was still
in progress, causing a segfault in left->getData(texture).

Another crash could occur when the 3D movie was looping back and not
all processes got the image, causing an invalid access in
_splitSideBySide(*image).

The first crash could be reproduced within seconds by opening a large
3D movie and moving the window around. The second was harder to
reproduce and its causes are not fully understood.

Due to the complexity of the movie decoding code there may still be a
unresolved underlying problem here; but at least this change
suppresses the crashes.
  • Loading branch information
Raphael Dumusc committed Aug 16, 2018
1 parent b4f55e1 commit 278bcc6
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
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)

* [264](https://github.com/BlueBrain/Tide/pull/264):
Fix a crash that could occur when playing large 3D movies.
* [261](https://github.com/BlueBrain/Tide/pull/261):
Tweak UI design and controls style.
* [260](https://github.com/BlueBrain/Tide/pull/260):
Expand Down
19 changes: 14 additions & 5 deletions tide/wall/datasources/MovieUpdater.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,14 @@

namespace
{
void _splitSideBySide(const FFMPEGPicture& image, PicturePtr& left,
PicturePtr& right)
auto _splitSideBySide(const FFMPEGPicture& image)
{
const auto width = image.getWidth() / 2;
const auto height = image.getHeight();
const auto format = image.getFormat();

left = std::make_shared<FFMPEGPicture>(width, height, format);
right = std::make_shared<FFMPEGPicture>(width, height, format);
auto left = std::make_shared<FFMPEGPicture>(width, height, format);
auto right = std::make_shared<FFMPEGPicture>(width, height, format);

const uint numTextures = (format == TextureFormat::rgba) ? 1 : 3 /*YUV*/;
for (uint texture = 0; texture < numTextures; ++texture)
Expand All @@ -75,6 +74,14 @@ void _splitSideBySide(const FFMPEGPicture& image, PicturePtr& left,
std::copy(input + targetWidth, input + lineWidth, outRight);
}
}
return std::make_pair(left, right);
}

auto _splitSideBySide(std::shared_ptr<FFMPEGPicture> image)
{
if (!image)
return std::make_pair(image, image);
return _splitSideBySide(*image);
}
}

Expand Down Expand Up @@ -162,6 +169,8 @@ ImagePtr MovieUpdater::getTileImage(const uint tileIndex,
if (loopBack)
image = _ffmpegMovie->getFrame(0.0);

// Warning: in rare cases image may still be null at this point

{
const QMutexLocker lock(&_mutex);
_currentPosition = _ffmpegMovie->getPosition();
Expand All @@ -172,7 +181,7 @@ ImagePtr MovieUpdater::getTileImage(const uint tileIndex,
}
if (_ffmpegMovie->isStereo())
{
_splitSideBySide(*image, _pictureLeftOrMono, _pictureRight);
std::tie(_pictureLeftOrMono, _pictureRight) = _splitSideBySide(image);
return view == deflect::View::right_eye ? _pictureRight
: _pictureLeftOrMono;
}
Expand Down

0 comments on commit 278bcc6

Please sign in to comment.