Skip to content
Snippets Groups Projects
Commit fbdc6a19 authored by Adrien Béraud's avatar Adrien Béraud Committed by Sébastien Blin
Browse files

video receive thread: fix race condition on displayMatrix_

Instances of std::shared_ptr are not thread-safe, even if the control block is.

Since AVBufer already has a refcounting system, use a unique_ptr instead,
and add a mutex to protect access during copy.

In practice, the mutex should almost never block since it's locked for
the minimum scope.

Change-Id: I5122e76dadb5da6c3738f8dc711698744b52315b
parent 35a0fcde
No related branches found
No related tags found
No related merge requests found
......@@ -24,11 +24,14 @@
#include <vector>
#include <map>
#include <string>
#include <memory>
extern "C" {
struct AVDictionary;
struct AVFrame;
struct AVPixFmtDescriptor;
struct AVBufferRef;
void av_buffer_unref(AVBufferRef **buf);
}
namespace jami {
......@@ -51,5 +54,11 @@ void fillWithBlack(AVFrame* frame);
void fillWithSilence(AVFrame* frame);
struct AVBufferRef_deleter {
void operator()(AVBufferRef* buf) const { av_buffer_unref(&buf); }
};
typedef std::unique_ptr<AVBufferRef, AVBufferRef_deleter> AVBufferPtr;
} // namespace libav_utils
} // namespace jami
......@@ -92,10 +92,16 @@ VideoReceiveThread::setup()
JAMI_DBG("[%p] Setupping video receiver", this);
videoDecoder_.reset(new MediaDecoder([this](const std::shared_ptr<MediaFrame>& frame) mutable {
if (auto displayMatrix = displayMatrix_)
libav_utils::AVBufferPtr displayMatrix;
{
std::lock_guard<std::mutex> l(rotationMtx_);
if (displayMatrix_)
displayMatrix.reset(av_buffer_ref(displayMatrix_.get()));
}
if (displayMatrix)
av_frame_new_side_data_from_buf(frame->pointer(),
AV_FRAME_DATA_DISPLAYMATRIX,
av_buffer_ref(displayMatrix.get()));
displayMatrix.release());
publishFrame(std::static_pointer_cast<VideoFrame>(frame));
}));
videoDecoder_->setResolutionChangedCallback([this](int width, int height) {
......@@ -303,15 +309,11 @@ VideoReceiveThread::getInfo() const
void
VideoReceiveThread::setRotation(int angle)
{
std::shared_ptr<AVBufferRef> displayMatrix {av_buffer_alloc(sizeof(int32_t) * 9),
[](AVBufferRef* buf) {
av_buffer_unref(&buf);
}};
if (displayMatrix) {
libav_utils::AVBufferPtr displayMatrix(av_buffer_alloc(sizeof(int32_t) * 9));
av_display_rotation_set(reinterpret_cast<int32_t*>(displayMatrix->data), angle);
std::lock_guard<std::mutex> l(rotationMtx_);
displayMatrix_ = std::move(displayMatrix);
}
}
} // namespace video
} // namespace jami
......@@ -31,6 +31,7 @@
#include "media_stream.h"
#include "threadloop.h"
#include "noncopyable.h"
#include "libav_utils.h"
#include <functional>
#include <map>
......@@ -106,7 +107,8 @@ private:
uint16_t mtu_;
int rotation_ {0};
std::shared_ptr<AVBufferRef> displayMatrix_;
std::mutex rotationMtx_;
libav_utils::AVBufferPtr displayMatrix_;
static int interruptCb(void* ctx);
static int readFunction(void* opaque, uint8_t* buf, int buf_size);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment