oak-team/oak-editor
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: OFX param instance lifecycle and thread-safety fixes

Browse Source 
- Fix SIGSEGV in PluginMisc.Keyer by linking Param::SetInstance to instances.
    Olive's custom param instances (IntegerInstance, DoubleInstance, etc.) were
    not passing the SetInstance pointer to the OpenFX HostSupport base class,
    leaving _paramSetInstance as nullptr. When Keyer called paramSetValue during
    createInstanceAction, the suite function dereferenced the null pointer in
    paramChangedByPlugin(). Now newParam() passes 'this' to every constructor.

  - Fix render-thread crash when OFX plugins set params during rendering.
    MinOFX calls paramSetValue inside createInstanceAction from the render
    thread. SubmitUndoCommand() used to push undo commands directly to
    UndoStack, which modifies QAction state (GUI-only). Added IsGuiThread()
    check: non-GUI threads execute redo_now() and discard the command without
    touching the undo stack.

  - Enable PluginMisc.MergeOver and PluginMisc.Keyer integration tests.
    MergeOver now supplies both Source and Bg textures; Keyer uses U16 format.
    Both pass in the full test suite.

  - Make ViewerQueue thread-safe with QMutex around AppendTimewise/PurgeBefore.
    Adds copy ctor and assignment to support mutex-per-instance semantics.
This commit is contained in:
Mike-Solar
2026-05-14 22:32:34 +08:00
parent 4aa4d59770
commit e85c6cf60a
6 changed files with 122 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/pluginSupport/OlivePluginInstance.cpp
+21 -23
View File
@@ -69,14 +69,6 @@ QString FormatOfxMessage(const char *format, va_list args)
return QString::fromUtf8(dynamic_buffer.constData());
}
bool IsGuiThread()
{
if (auto *app = QCoreApplication::instance()) {
return QThread::currentThread() == app->thread();
}
return true;
}
const std::string &FieldOrderForParams(const VideoParams &params)
{
switch (params.interlacing()) {
@@ -329,36 +321,36 @@ OlivePluginInstance::newParam(const std::string &name,
const std::string &type = desc.getType();
if (type == kOfxParamTypeInteger) {
return new IntegerInstance(node_, desc);
return new IntegerInstance(node_, desc, this);
} else if (type == kOfxParamTypeDouble) {
return new DoubleInstance(node_, name, desc);
return new DoubleInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeBoolean) {
return new BooleanInstance(node_, name, desc);
return new BooleanInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeChoice) {
return new ChoiceInstance(node_, name, desc);
return new ChoiceInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeString) {
return new StringInstance(node_, name, desc);
return new StringInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeRGBA) {
return new RGBAInstance(node_, name, desc);
return new RGBAInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeRGB) {
return new RGBInstance(node_, name, desc);
return new RGBInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeDouble2D) {
return new Double2DInstance(node_, name, desc);
return new Double2DInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeInteger2D) {
return new Integer2DInstance(node_, name, desc);
return new Integer2DInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeDouble3D) {
return new Double3DInstance(node_, name, desc);
return new Double3DInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeInteger3D) {
return new Integer3DInstance(node_, name, desc);
return new Integer3DInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeCustom ||
type == kOfxParamTypeBytes) {
return new CustomInstance(node_, name, desc);
return new CustomInstance(node_, name, desc, this);
} else if (type == kOfxParamTypeGroup) {
return new GroupInstance(desc);
return new GroupInstance(desc, this);
} else if (type == kOfxParamTypePage) {
return new PageInstance(desc);
return new PageInstance(desc, this);
} else if (type == kOfxParamTypePushButton) {
return new PushbuttonInstance(node_, name, desc);
return new PushbuttonInstance(node_, name, desc, this);
}
return nullptr; // 未实现的类型
@@ -431,6 +423,12 @@ void OlivePluginInstance::SubmitUndoCommand(UndoCommand *command,
return;
}
if (!IsGuiThread()) {
command->redo_now();
delete command;
return;
}
Core::instance()->undo_stack()->push(command, label);
}
app/pluginSupport/OlivePluginInstance.h
+12 -1
View File
@@ -26,12 +26,23 @@
#include "undo/undocommand.h"
#include <map>
#include <QCoreApplication>
#include <QPointer>
#include <QThread>
#include <qcontainerfwd.h>
#include <qlist.h>
namespace olive {
inline bool IsGuiThread()
{
if (auto *app = QCoreApplication::instance()) {
return QThread::currentThread() == app->thread();
}
return true;
}
class ProgressDialog;
namespace plugin{
namespace plugin {
class PluginNode;
enum class ErrorType{
Error,
app/pluginSupport/paraminstance.cpp
+6
View File
@@ -43,6 +43,12 @@ void SubmitUndoCommand(const std::shared_ptr<PluginNode> &node,
}
}
if (!IsGuiThread()) {
command->redo_now();
delete command;
return;
}
Core::instance()->undo_stack()->push(command, label);
}
}
app/pluginSupport/paraminstance.h
+45 -30
View File
@@ -85,8 +85,9 @@ protected:
OFX::Host::Param::Descriptor *_descriptor;
public:
PushbuttonInstance(std::shared_ptr<PluginNode> effect, const std::string &name,
OFX::Host::Param::Descriptor &descriptor)
: OFX::Host::Param::PushbuttonInstance(descriptor)
OFX::Host::Param::Descriptor &descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::PushbuttonInstance(descriptor, paramSet)
, node(effect)
{
_descriptor = &descriptor;
@@ -106,8 +107,9 @@ protected:
bool has_value_ = false;
int value_ = 0;
public:
IntegerInstance(std::shared_ptr<PluginNode>node, OFX::Host::Param::Descriptor &descriptor)
: OFX::Host::Param::IntegerInstance(descriptor)
IntegerInstance(std::shared_ptr<PluginNode>node, OFX::Host::Param::Descriptor &descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::IntegerInstance(descriptor, paramSet)
, _node(node)
, _descriptor(descriptor)
{}
@@ -191,8 +193,9 @@ protected:
bool has_value_ = false;
double value_ = 0.0;
public:
DoubleInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::DoubleInstance(descriptor)
DoubleInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::DoubleInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -305,8 +308,9 @@ protected:
.getIntProperty(kOfxParamPropDefault) != 0;
}
public:
BooleanInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::BooleanInstance(descriptor)
BooleanInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::BooleanInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -392,8 +396,9 @@ protected:
bool has_value_ = false;
int value_ = 0;
public:
ChoiceInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::ChoiceInstance(descriptor)
ChoiceInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::ChoiceInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -471,8 +476,9 @@ protected:
bool has_value_ = false;
double value_[4] = {0.0, 0.0, 0.0, 0.0};
public:
RGBAInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::RGBAInstance(descriptor)
RGBAInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::RGBAInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -581,8 +587,9 @@ protected:
bool has_value_ = false;
double value_[3] = {0.0, 0.0, 0.0};
public:
RGBInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::RGBInstance(descriptor)
RGBInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::RGBInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -682,8 +689,9 @@ protected:
bool has_value_ = false;
double value_[2] = {0.0, 0.0};
public:
Double2DInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::Double2DInstance(descriptor)
Double2DInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::Double2DInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -798,8 +806,9 @@ protected:
bool has_value_ = false;
int value_[2] = {0, 0};
public:
Integer2DInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::Integer2DInstance(descriptor)
Integer2DInstance(std::shared_ptr<PluginNode> effect, const std::string& name, OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::Integer2DInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -889,8 +898,9 @@ protected:
double value_[3] = {0.0, 0.0, 0.0};
public:
Double3DInstance(std::shared_ptr<PluginNode> effect, const std::string& name,
OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::Double3DInstance(descriptor)
OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::Double3DInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -1018,8 +1028,9 @@ protected:
int value_[3] = {0, 0, 0};
public:
Integer3DInstance(std::shared_ptr<PluginNode> effect, const std::string& name,
OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::Integer3DInstance(descriptor)
OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::Integer3DInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -1117,8 +1128,9 @@ protected:
std::string value_;
public:
StringInstance(std::shared_ptr<PluginNode> effect, const std::string& name,
OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::StringInstance(descriptor)
OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::StringInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -1198,8 +1210,9 @@ protected:
std::string value_;
public:
CustomInstance(std::shared_ptr<PluginNode> effect, const std::string& name,
OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::CustomInstance(descriptor)
OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::CustomInstance(descriptor, paramSet)
, node(effect)
, _descriptor(descriptor)
{
@@ -1280,16 +1293,18 @@ public:
class GroupInstance : public OFX::Host::Param::GroupInstance {
public:
GroupInstance(OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::GroupInstance(descriptor)
GroupInstance(OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::GroupInstance(descriptor, paramSet)
{
}
};
class PageInstance : public OFX::Host::Param::PageInstance {
public:
PageInstance(OFX::Host::Param::Descriptor& descriptor)
: OFX::Host::Param::PageInstance(descriptor)
PageInstance(OFX::Host::Param::Descriptor& descriptor,
OFX::Host::Param::SetInstance *paramSet = nullptr)
: OFX::Host::Param::PageInstance(descriptor, paramSet)
{
}
};
app/widget/viewer/viewerqueue.h
+16 -1
View File
@@ -23,6 +23,8 @@
#define VIEWERQUEUE_H
#include <QVariant>
#include <QMutex>
#include <QMutexLocker>
#include "codec/frame.h"
@@ -36,10 +38,19 @@ struct ViewerPlaybackFrame {
class ViewerQueue : public std::list<ViewerPlaybackFrame> {
public:
ViewerQueue() = default;
ViewerQueue() : mutex_(new QMutex()) {}
ViewerQueue(const ViewerQueue &other)
: std::list<ViewerPlaybackFrame>(other), mutex_(new QMutex()) {}
~ViewerQueue() { delete mutex_; }
ViewerQueue &operator=(const ViewerQueue &other)
{
std::list<ViewerPlaybackFrame>::operator=(other);
return *this;
}
void AppendTimewise(const ViewerPlaybackFrame &f, int playback_speed)
{
QMutexLocker locker(mutex_);
if (this->empty() ||
(this->back().timestamp < f.timestamp) == (playback_speed > 0)) {
this->push_back(f);
@@ -55,12 +66,16 @@ public:
void PurgeBefore(const rational &time, int playback_speed)
{
QMutexLocker locker(mutex_);
while (!this->empty() &&
((playback_speed > 0 && this->front().timestamp < time) ||
(playback_speed < 0 && this->front().timestamp > time))) {
this->pop_front();
}
}
private:
QMutex *mutex_;
};
}
tests/gtest/plugin_ofx_misc_test.cpp
+22 -12
View File
@@ -426,12 +426,18 @@ TEST(PluginMisc, MergeOver)
GTEST_SKIP() << "OFX integration test not enabled";
}
// FIXME: Merge plugin crashes during render. Needs investigation.
// The crash happens inside PluginRenderer::RenderPlugin, likely due to:
// 1. Multi-input plugin handling issues
// 2. Missing parameter initialization for the merge operation
// 3. Clip format compatibility issues between A and B inputs
GTEST_SKIP() << "Merge plugin crashes - needs fix in multi-input handling";
VideoParams params(320, 240, core::PixelFormat::U8, 4);
TexturePtr input = CreateSolidTexture(params, 0x80);
ASSERT_NE(input, nullptr);
NodeValueRow row;
row.insert(QString::fromStdString(kOfxImageEffectSimpleSourceClipName),
NodeValue(NodeValue::kTexture, input));
row.insert(QStringLiteral("Bg"),
NodeValue(NodeValue::kTexture, input));
bool result = RenderPlugin("net.sf.openfx.MergePlugin", params, row, true);
EXPECT_TRUE(result) << "Merge plugin should produce output";
}
// ============================================================================
@@ -444,12 +450,16 @@ TEST(PluginMisc, Keyer)
GTEST_SKIP() << "OFX integration test not enabled";
}
// FIXME: Keyer plugin crashes during render. Needs investigation.
// The crash happens inside PluginRenderer::RenderPlugin. Possible causes:
// 1. Keyer requires additional optional inputs (Bg, InM, OutM) to be explicitly unset
// 2. Parameter initialization issues (keyColor, mode, etc.)
// 3. The plugin explicitly disables UByte support, U16/F32 may need special handling
GTEST_SKIP() << "Keyer plugin crashes - needs investigation";
VideoParams params(320, 240, core::PixelFormat::U16, 4);
TexturePtr input = CreateSolidTexture(params, 0x80);
ASSERT_NE(input, nullptr);
NodeValueRow row;
row.insert(QString::fromStdString(kOfxImageEffectSimpleSourceClipName),
NodeValue(NodeValue::kTexture, input));
bool result = RenderPlugin("net.sf.openfx.KeyerPlugin", params, row, true);
EXPECT_TRUE(result) << "Keyer plugin should produce output";
}
// ============================================================================