fix(renderer): macOS TBDR cross-context sync and OFX instance thread-safety
This commit fixes crashes (SIGSEGV in CImg blur_bilateral) and black-frame corruption artifacts during playback and scrubbing on macOS Apple Silicon. Root cause analysis: 1. macOS uses Tile-Based Deferred Rendering (TBDR). glFlush() does not guarantee tile memory writeback, causing glReadPixels to read incomplete tiles (black/corrupted frames) and cache them to disk. 2. Olive uses multiple shared OpenGL contexts (RenderProcessor contexts vs. thread-local PluginRenderer context). glFinish() only waits for the current context, not the shared context that produced the texture. CPU readback in PluginRenderer could read partially-rendered tiles. 3. OlivePluginInstance and OliveClipInstance are not thread-safe. Concurrent RenderProcessors could corrupt internal QMap/images_ and params_ via setInputTexture/renderAction races. Fixes: - OpenGLRenderer::Flush() on macOS now uses glFinish() unconditionally. - OpenGLRenderer::DownloadFromTexture() and OpenGLRenderer::Blit() insert glFinish() before readback/detach to ensure tile writeback completes. - PluginRenderer::RenderPlugin() now acquires a per-instance mutex to serialize concurrent OFX render calls. - Before CPU readback in PluginRenderer, flush the renderer that originally produced each input texture, ensuring cross-context synchronization. - RenderProcessor::ProcessVideoFootage() flushes after BlitColorManaged. - Add black-frame detection in ProcessVideoCacheJob() to auto-purge TBDR- corrupted cache files. - Add diagnostic qDebug() logging in viewer, decoder, renderer, and plugin paths to aid future debugging.
This commit is contained in:
@@ -349,6 +349,21 @@ TexturePtr FFmpegDecoder::RetrieveVideoInternal(const RetrieveVideoParams &p)
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
// Diagnostic: check if decoded frame is all black
|
||||
bool all_black = true;
|
||||
if (f->data[0]) {
|
||||
int check_rows = std::min(f->height, 8);
|
||||
int check_bytes = check_rows * f->linesize[0];
|
||||
for (int i = 0; i < check_bytes; ++i) {
|
||||
if (f->data[0][i] != 0) {
|
||||
all_black = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
qDebug() << "[DECODER] RetrieveVideoInternal time=" << p.time.toDouble()
|
||||
<< "format=" << static_cast<int>(f->format) << "black=" << all_black;
|
||||
|
||||
// Finally, perform any GPU processing required
|
||||
TexturePtr texture = ProcessFrameIntoTexture(f, p, original);
|
||||
|
||||
|
||||
@@ -26,6 +26,7 @@
|
||||
#include "undo/undocommand.h"
|
||||
|
||||
#include <map>
|
||||
#include <mutex>
|
||||
#include <QCoreApplication>
|
||||
#include <QPointer>
|
||||
#include <QThread>
|
||||
@@ -223,6 +224,10 @@ private:
|
||||
bool progress_cancelled_ = false;
|
||||
bool progress_active_ = false;
|
||||
bool open_gl_enabled_ = false;
|
||||
public:
|
||||
std::mutex& mutex() { return mutex_; }
|
||||
private:
|
||||
std::mutex mutex_;
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -370,6 +370,9 @@ void OpenGLRenderer::DownloadFromTexture(const QVariant &id,
|
||||
|
||||
functions_->glPixelStorei(GL_PACK_ROW_LENGTH, linesize);
|
||||
|
||||
// Ensure all rendering is complete before reading back on TBDR architectures (macOS)
|
||||
functions_->glFinish();
|
||||
|
||||
{
|
||||
PRINT_GL_ERRORS;
|
||||
functions_->glReadPixels(0, 0, p.effective_width(),
|
||||
@@ -392,7 +395,14 @@ void OpenGLRenderer::Flush()
|
||||
if (OLIVE_CONFIG("UseGLFinish").toBool()) {
|
||||
functions_->glFinish();
|
||||
} else {
|
||||
#if defined(Q_OS_MAC)
|
||||
// macOS uses Tile-Based Deferred Rendering (TBDR). glFlush() does not
|
||||
// guarantee that tile memory has been written back to texture memory.
|
||||
// Using glFinish() prevents partial tile corruption ("black ink" artifacts).
|
||||
functions_->glFinish();
|
||||
#else
|
||||
functions_->glFlush();
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -32,6 +32,7 @@
|
||||
#include <cmath>
|
||||
#include <cstdint>
|
||||
#include <cstring>
|
||||
#include <mutex>
|
||||
#include <qtypes.h>
|
||||
#include <string>
|
||||
#include <vector>
|
||||
@@ -1378,6 +1379,21 @@ void olive::plugin::PluginRenderer::RenderPlugin(TexturePtr src, olive::plugin::
|
||||
if (!instance) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Lock the plugin instance to prevent concurrent access from multiple
|
||||
// RenderProcessors. OlivePluginInstance (and its OliveClipInstance) are not
|
||||
// thread-safe; concurrent calls to setInputTexture/renderAction can corrupt
|
||||
// internal QMap/images_ and params_, leading to invalid pointers being
|
||||
// passed to CImg and subsequent SIGSEGV.
|
||||
std::mutex *instance_mutex = nullptr;
|
||||
if (auto *olive_inst = dynamic_cast<olive::plugin::OlivePluginInstance *>(instance)) {
|
||||
instance_mutex = &olive_inst->mutex();
|
||||
} else {
|
||||
static std::mutex fallback_mutex;
|
||||
instance_mutex = &fallback_mutex;
|
||||
}
|
||||
std::lock_guard<std::mutex> instance_lock(*instance_mutex);
|
||||
|
||||
bool supports_opengl = false;
|
||||
#ifdef OFX_SUPPORTS_OPENGLRENDER
|
||||
const std::string &gl_supported =
|
||||
@@ -1552,6 +1568,16 @@ void olive::plugin::PluginRenderer::RenderPlugin(TexturePtr src, olive::plugin::
|
||||
|
||||
// set correct format for input
|
||||
|
||||
// Ensure all input textures are fully rendered before CPU readback.
|
||||
// BlitColorManaged may have executed in a different shared OpenGL context;
|
||||
// glFinish() in our context does NOT wait for commands in that context,
|
||||
// so we must flush the renderer that actually produced the texture.
|
||||
for (const auto &entry : input_textures) {
|
||||
if (entry.second && entry.second->renderer()) {
|
||||
entry.second->renderer()->Flush();
|
||||
}
|
||||
}
|
||||
|
||||
auto &descriptor = instance->getDescriptor();
|
||||
for (const auto &entry : input_clips) {
|
||||
if (entry.first == kOfxImageEffectOutputClipName) {
|
||||
@@ -1668,6 +1694,10 @@ void olive::plugin::PluginRenderer::RenderPlugin(TexturePtr src, olive::plugin::
|
||||
|
||||
// render a frame
|
||||
const char *render_field = GetRenderFieldForParams(output_params);
|
||||
qDebug() << "[PLUGIN] RenderPlugin use_opengl=" << use_opengl
|
||||
<< "time=" << frame
|
||||
<< "plugin=" << PluginIdForInstance(instance)
|
||||
<< "dest_valid=" << (destination ? destination->id().isValid() : false);
|
||||
stat = instance->renderAction(frame, render_field, renderWindow, renderScale,
|
||||
true, interactive, interactive);
|
||||
if (stat != kOfxStatOK && stat != kOfxStatReplyDefault) {
|
||||
@@ -1708,6 +1738,19 @@ void olive::plugin::PluginRenderer::RenderPlugin(TexturePtr src, olive::plugin::
|
||||
renderScale, true, interactive);
|
||||
return;
|
||||
}
|
||||
// Diagnostic: peek at first few pixels
|
||||
void *img_data = output_image->getPointerProperty(kOfxImagePropData);
|
||||
bool img_black = true;
|
||||
if (img_data) {
|
||||
float *f = static_cast<float *>(img_data);
|
||||
for (int i = 0; i < 16; ++i) {
|
||||
if (f[i] != 0.0f) {
|
||||
img_black = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
qDebug() << "[PLUGIN] output_image black=" << img_black << "plugin=" << PluginIdForInstance(instance);
|
||||
} else {
|
||||
if (!destination || !destination->id().isValid()) {
|
||||
#ifdef OFX_SUPPORTS_OPENGLRENDER
|
||||
@@ -1753,6 +1796,7 @@ void olive::plugin::PluginRenderer::RenderPlugin(TexturePtr src, olive::plugin::
|
||||
} else {
|
||||
// OpenGL path: plugin has already rendered directly into the destination
|
||||
// texture via FBO/GL. No CPU readback or conversion needed.
|
||||
qDebug() << "[PLUGIN] OpenGL path done, returning directly";
|
||||
#ifdef OFX_SUPPORTS_OPENGLRENDER
|
||||
DetachOutputTexture();
|
||||
instance->contextDetachedAction();
|
||||
|
||||
@@ -151,6 +151,19 @@ FramePtr RenderProcessor::GenerateFrame(TexturePtr texture,
|
||||
render_ctx_->DownloadFromTexture(texture->id(), texture->params(),
|
||||
frame->data(),
|
||||
frame->linesize_pixels());
|
||||
|
||||
// Diagnostic: check if downloaded frame is all black
|
||||
bool all_black = true;
|
||||
const uint8_t *pixels = reinterpret_cast<const uint8_t *>(frame->data());
|
||||
size_t total_bytes = frame->allocated_size();
|
||||
for (size_t i = 0; i < std::min(total_bytes, size_t(1024)); ++i) {
|
||||
if (pixels[i] != 0) {
|
||||
all_black = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
qDebug() << "[RENDER] GenerateFrame DownloadFromTexture all_black="
|
||||
<< all_black << "total_bytes=" << total_bytes;
|
||||
}
|
||||
|
||||
return frame;
|
||||
@@ -196,6 +209,9 @@ void RenderProcessor::Run()
|
||||
}
|
||||
|
||||
TexturePtr texture = GenerateTexture(time, frame_length);
|
||||
qDebug() << "[RENDER] GenerateTexture time=" << time.toDouble()
|
||||
<< "tex_null=" << (texture == nullptr)
|
||||
<< "tex_dummy=" << (texture ? texture->IsDummy() : true);
|
||||
|
||||
if (!render_ctx_) {
|
||||
ticket_->Finish();
|
||||
@@ -219,6 +235,7 @@ void RenderProcessor::Run()
|
||||
if (HeardCancel()) {
|
||||
// Finish cancelled ticket with nothing since we can't guarantee the frame we generated
|
||||
// is actually "complete
|
||||
qDebug() << "[RENDER] HeardCancel, finishing empty";
|
||||
ticket_->Finish();
|
||||
} else {
|
||||
FramePtr frame;
|
||||
@@ -252,7 +269,7 @@ void RenderProcessor::Run()
|
||||
}
|
||||
|
||||
render_ctx_->Flush();
|
||||
|
||||
qDebug() << "[RENDER] Finishing with texture";
|
||||
ticket_->Finish(QVariant::fromValue(texture));
|
||||
} else {
|
||||
ticket_->Finish(QVariant::fromValue(frame));
|
||||
@@ -486,6 +503,9 @@ void RenderProcessor::ProcessVideoFootage(TexturePtr destination,
|
||||
}
|
||||
|
||||
render_ctx_->BlitColorManaged(job, destination.get());
|
||||
// macOS TBDR: ensure tile writeback completes before the texture
|
||||
// is read back in a potentially different shared OpenGL context.
|
||||
render_ctx_->Flush();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -711,6 +731,27 @@ TexturePtr RenderProcessor::ProcessVideoCacheJob(const CacheJob *val)
|
||||
{
|
||||
FramePtr frame = FrameHashCache::LoadCacheFrame(val->GetFilename());
|
||||
if (frame) {
|
||||
// Auto-detect and discard black/empty cached frames (macOS TBDR artifact)
|
||||
bool all_black = true;
|
||||
if (frame->data() && frame->allocated_size() > 0) {
|
||||
const uint8_t *pixels = reinterpret_cast<const uint8_t *>(frame->data());
|
||||
size_t alloc_size = static_cast<size_t>(frame->allocated_size());
|
||||
size_t check_bytes = std::min(alloc_size, size_t(4096));
|
||||
for (size_t i = 0; i < check_bytes; ++i) {
|
||||
if (pixels[i] != 0) {
|
||||
all_black = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (all_black) {
|
||||
qWarning() << "[CACHE] Discarding black cached frame:" << val->GetFilename()
|
||||
<< "time=" << frame->timestamp().toDouble()
|
||||
<< "size=" << frame->allocated_size();
|
||||
QFile::remove(val->GetFilename());
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
TexturePtr tex = CreateTexture(frame->video_params());
|
||||
if (tex) {
|
||||
tex->Upload(frame->data(), frame->linesize_pixels());
|
||||
|
||||
@@ -205,6 +205,9 @@ void ViewerWidget::TimeChangedEvent(const rational &time)
|
||||
|
||||
if (GetConnectedNode() && last_time_ != time) {
|
||||
if (!IsPlaying()) {
|
||||
qDebug() << "[VIEWER] TimeChanged seeking to" << time.toDouble()
|
||||
<< "frame_exists=" << FrameExistsAtTime(time)
|
||||
<< "might_be_still=" << ViewerMightBeAStill();
|
||||
UpdateTextureFromNode();
|
||||
|
||||
PushScrubbedAudio();
|
||||
@@ -1445,16 +1448,30 @@ void ViewerWidget::WindowAboutToClose()
|
||||
void ViewerWidget::RendererGeneratedFrame()
|
||||
{
|
||||
RenderTicketWatcher *ticket = static_cast<RenderTicketWatcher *>(sender());
|
||||
rational t = ticket->property("time").value<rational>();
|
||||
bool has_result = ticket->HasResult();
|
||||
qDebug() << "[VIEWER] RendererGeneratedFrame time=" << t.toDouble()
|
||||
<< "has_result=" << has_result
|
||||
<< "nonqueue_size=" << nonqueue_watchers_.size();
|
||||
|
||||
if (ticket->HasResult()) {
|
||||
if (nonqueue_watchers_.contains(ticket)) {
|
||||
while (!nonqueue_watchers_.isEmpty()) {
|
||||
// Pop frames that are "old"
|
||||
if (nonqueue_watchers_.takeFirst() == ticket) {
|
||||
break;
|
||||
}
|
||||
if (nonqueue_watchers_.contains(ticket)) {
|
||||
while (!nonqueue_watchers_.isEmpty()) {
|
||||
// Pop frames that are "old"
|
||||
if (nonqueue_watchers_.takeFirst() == ticket) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (ticket->HasResult()) {
|
||||
QVariant v = ticket->Get();
|
||||
bool is_tex = v.canConvert<TexturePtr>();
|
||||
bool is_frame = v.canConvert<FramePtr>();
|
||||
TexturePtr tex = v.value<TexturePtr>();
|
||||
qDebug() << "[VIEWER] SetDisplayImage time=" << t.toDouble()
|
||||
<< "is_texture=" << is_tex
|
||||
<< "is_frame=" << is_frame
|
||||
<< "tex_null=" << (tex == nullptr)
|
||||
<< "tex_dummy=" << (tex ? tex->IsDummy() : true);
|
||||
SetDisplayImage(ticket->GetTicket());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -382,11 +382,15 @@ void ViewerDisplayWidget::OnPaint()
|
||||
bg_color.blueF());
|
||||
|
||||
// We only draw if we have a pipeline
|
||||
qDebug() << "[VIEWER] OnPaint push_mode=" << push_mode_
|
||||
<< "color_service=" << (color_service() != nullptr)
|
||||
<< "has_load_frame=" << !load_frame_.isNull();
|
||||
if (push_mode_ != kPushNull) {
|
||||
// Draw texture through color transform
|
||||
VideoParams device_params = GetViewportParams();
|
||||
|
||||
if (push_mode_ == kPushBlank) {
|
||||
qDebug() << "[VIEWER] OnPaint drawing blank";
|
||||
DrawBlank(device_params);
|
||||
} else if (color_service()) {
|
||||
if (FramePtr frame = load_frame_.value<FramePtr>()) {
|
||||
@@ -490,7 +494,10 @@ void ViewerDisplayWidget::OnPaint()
|
||||
ctj.SetForceOpaque(true);
|
||||
|
||||
renderer()->BlitColorManaged(ctj, device_params);
|
||||
qDebug() << "[VIEWER] OnPaint BlitColorManaged done";
|
||||
}
|
||||
} else {
|
||||
qDebug() << "[VIEWER] OnPaint no color_service, skipping texture draw";
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user