diff --git a/app/dialog/export/export.cpp b/app/dialog/export/export.cpp index d2dc9163b..20c280963 100644 --- a/app/dialog/export/export.cpp +++ b/app/dialog/export/export.cpp @@ -462,6 +462,16 @@ ExportDialog::ExportDialog(OakEngineNode *viewer_node, bool stills_only_mode, subtitle_tab_->setEnabled(subtitles_enabled_->isChecked()); } +ExportDialog::~ExportDialog() +{ + // Raw C-API subscription carries `this` as userdata; not covered by + // Qt's auto-disconnect. + if (viewer_sub_ > 0) { + oakengine_event_unsubscribe(viewer_sub_); + viewer_sub_ = 0; + } +} + Rational ExportDialog::get_selected_timebase() const { return video_tab_->get_selected_frame_rate().flipped(); diff --git a/app/dialog/export/export.h b/app/dialog/export/export.h index ff3e2a43e..01c0743df 100644 --- a/app/dialog/export/export.h +++ b/app/dialog/export/export.h @@ -49,6 +49,7 @@ public: : ExportDialog(viewer_node, false, parent) { } + ~ExportDialog() override; Rational get_selected_timebase() const; void set_selected_timebase(const Rational &r); diff --git a/app/widget/nodeparamview/nodeparamviewconnectedlabel.cpp b/app/widget/nodeparamview/nodeparamviewconnectedlabel.cpp index bb6546945..d3719b37e 100644 --- a/app/widget/nodeparamview/nodeparamviewconnectedlabel.cpp +++ b/app/widget/nodeparamview/nodeparamviewconnectedlabel.cpp @@ -110,6 +110,14 @@ NodeParamViewConnectedLabel::NodeParamViewConnectedLabel(const oak::Input &input &NodeParamViewConnectedLabel::set_value_tree_visible); } +NodeParamViewConnectedLabel::~NodeParamViewConnectedLabel() +{ + // Raw C-API subscription carries `this` as userdata; not covered by + // Qt's auto-disconnect. Unsubscribe or the next playhead event calls + // into a dead object. + set_viewer_node(nullptr); +} + void NodeParamViewConnectedLabel::set_viewer_node(OakEngineNode *viewer) { if (viewer_) { diff --git a/app/widget/nodeparamview/nodeparamviewconnectedlabel.h b/app/widget/nodeparamview/nodeparamviewconnectedlabel.h index f8f92b7e2..51e9d264f 100644 --- a/app/widget/nodeparamview/nodeparamviewconnectedlabel.h +++ b/app/widget/nodeparamview/nodeparamviewconnectedlabel.h @@ -37,6 +37,7 @@ class NodeParamViewConnectedLabel : public QWidget { public: NodeParamViewConnectedLabel(const oak::Input &input, QWidget *parent = nullptr); + ~NodeParamViewConnectedLabel() override; void set_viewer_node(OakEngineNode *viewer); diff --git a/app/widget/nodeparamview/nodeparamviewitem.cpp b/app/widget/nodeparamview/nodeparamviewitem.cpp index 6318c7d32..6c9d47505 100644 --- a/app/widget/nodeparamview/nodeparamviewitem.cpp +++ b/app/widget/nodeparamview/nodeparamviewitem.cpp @@ -37,8 +37,9 @@ static oak::Input ResolveGroupInput(const oak::Input &input) char input_id[256]; int element = input.element(); const QByteArray utf = input.input_id().toUtf8(); - memcpy(input_id, utf.constData(), qMin(sizeof(input_id) - 1, utf.size())); - input_id[sizeof(input_id) - 1] = '\0'; + const int len = qMin(sizeof(input_id) - 1, utf.size()); + memcpy(input_id, utf.constData(), len); + input_id[len] = '\0'; // WRAPPER-GAP: oakengine_group_resolve_input (group API has no oak:: wrapper) if (oakengine_group_resolve_input( node, input_id, element, diff --git a/app/widget/nodeparamview/nodeparamviewkeyframecontrol.cpp b/app/widget/nodeparamview/nodeparamviewkeyframecontrol.cpp index 48d14af95..e4c669bbe 100644 --- a/app/widget/nodeparamview/nodeparamviewkeyframecontrol.cpp +++ b/app/widget/nodeparamview/nodeparamviewkeyframecontrol.cpp @@ -157,6 +157,20 @@ NodeParamViewKeyframeControl::NodeParamViewKeyframeControl(bool right_align, show_buttons_from_keyframe_enable(false); } +NodeParamViewKeyframeControl::~NodeParamViewKeyframeControl() +{ + // Raw C-API subscription carries `this` as userdata; it is not covered + // by Qt's auto-disconnect. Without this, a playhead event delivered + // after destruction calls update_state() on a dead object. + if (viewer_sub_ > 0) { + oakengine_event_unsubscribe(viewer_sub_); + viewer_sub_ = 0; + } + // Drop the keyframe_* bridge subscriptions too (same raw-userdata + // mechanism underneath). + set_input(oak::Input()); +} + void NodeParamViewKeyframeControl::set_input(const oak::Input &input) { if (input_.is_valid()) { diff --git a/app/widget/nodeparamview/nodeparamviewkeyframecontrol.h b/app/widget/nodeparamview/nodeparamviewkeyframecontrol.h index 0a20882e6..33852f696 100644 --- a/app/widget/nodeparamview/nodeparamviewkeyframecontrol.h +++ b/app/widget/nodeparamview/nodeparamviewkeyframecontrol.h @@ -41,6 +41,7 @@ public: : NodeParamViewKeyframeControl(true, parent) { } + ~NodeParamViewKeyframeControl() override; const oak::Input &get_connected_input() const { diff --git a/app/widget/nodeparamview/nodeparamviewwidgetbridge.cpp b/app/widget/nodeparamview/nodeparamviewwidgetbridge.cpp index f9460a253..15bf47f81 100644 --- a/app/widget/nodeparamview/nodeparamviewwidgetbridge.cpp +++ b/app/widget/nodeparamview/nodeparamviewwidgetbridge.cpp @@ -142,9 +142,9 @@ static bool ResolveGroupInput(oak::Input *input) char input_id[256]; int element = input->element(); const QByteArray utf = input->input_id().toUtf8(); - memcpy(input_id, utf.constData(), - qMin(sizeof(input_id) - 1, utf.size())); - input_id[sizeof(input_id) - 1] = '\0'; + const int len = qMin(sizeof(input_id) - 1, utf.size()); + memcpy(input_id, utf.constData(), len); + input_id[len] = '\0'; if (!oakengine_node_group_get_inner(&node, input_id, sizeof(input_id), &element)) { return false; diff --git a/app/widget/nodeview/nodeviewcontext.cpp b/app/widget/nodeview/nodeviewcontext.cpp index 85ac1eeba..fe9b18d5e 100644 --- a/app/widget/nodeview/nodeviewcontext.cpp +++ b/app/widget/nodeview/nodeviewcontext.cpp @@ -255,6 +255,12 @@ bool NodeViewContext::child_input_disconnected(OakEngineNode *output, for (int i = 0; i < edges_.size(); i++) { NodeViewEdge *e = edges_.at(i); if (e->output() == oak::Node(output) && e->input() == input) { + if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) { + qWarning("EDGE-DEBUG: edge removed: %p -> %p (%s,%d) ctx=%p", + (void *)output, (void *)input.node_handle(), + qPrintable(input.input_id()), input.element(), + (void *)this); + } delete e; edges_.removeAt(i); return true; @@ -413,6 +419,11 @@ void NodeViewContext::mousePressEvent(QGraphicsSceneMouseEvent *event) void NodeViewContext::add_node_internal(OakEngineNode *node, NodeViewItem *item) { + if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) { + qWarning("EDGE-DEBUG: node added to ctx %p: %p", (void *)this, + (void *)node); + } + node_subs_[node].append(bridge_->subscribe( reinterpret_cast(node), OAKENGINE_EVENT_NODE_INPUT_CONNECTED)); @@ -436,6 +447,11 @@ void NodeViewContext::add_node_internal(OakEngineNode *node, NodeViewItem *item) oak::Input ai(conn.node.handle(), conn.input_id, conn.element); add_edge_internal(node, ai, item, other_item->get_item_for_input(ai)); + } else if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) { + qWarning("EDGE-DEBUG: out-edge skipped (no item for %p): %p -> %p (%s) ctx=%p", + (void *)conn.node.handle(), (void *)node, + (void *)conn.node.handle(), + qPrintable(conn.input_id), (void *)this); } } } @@ -449,6 +465,11 @@ void NodeViewContext::add_node_internal(OakEngineNode *node, NodeViewItem *item) oak::Input ai(conn.node.handle(), conn.input_id, conn.element); add_edge_internal(conn.source_node.handle(), ai, other_item, item->get_item_for_input(ai)); + } else if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) { + qWarning("EDGE-DEBUG: in-edge skipped (no item for %p): %p -> %p (%s) ctx=%p", + (void *)conn.source_node.handle(), + (void *)conn.source_node.handle(), (void *)node, + qPrintable(conn.input_id), (void *)this); } } } @@ -461,6 +482,12 @@ void NodeViewContext::add_edge_internal(OakEngineNode *output, const oak::Input return; } + if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) { + qWarning("EDGE-DEBUG: edge added: %p -> %p (%s,%d) ctx=%p", (void *)output, + (void *)input.node_handle(), qPrintable(input.input_id()), + input.element(), (void *)this); + } + NodeViewEdge *edge_ui = new NodeViewEdge(oak::Node(output), input, from, to, this); edge_ui->adjust(); diff --git a/app/widget/viewer/viewer.cpp b/app/widget/viewer/viewer.cpp index 00b0871a2..3d26d6ee9 100644 --- a/app/widget/viewer/viewer.cpp +++ b/app/widget/viewer/viewer.cpp @@ -110,7 +110,7 @@ static SampleBuffer preview_req_to_sample_buffer(OakEnginePreviewRequest *req) oakcore_audioparams_free(ap); // Determine sample count from first channel - int n = oakengine_preview_request_get_audio_samples(req, 0, nullptr, 1 << 30); + int n = oakengine_preview_request_get_audio_sample_count(req); if (n > 0) { oakcore_samplebuffer_set_sample_count(sb, size_t(n)); oakcore_samplebuffer_allocate(sb); diff --git a/docs/zh/investigation-edge-display-and-playback.md b/docs/zh/investigation-edge-display-and-playback.md new file mode 100644 index 000000000..2be9d96ef --- /dev/null +++ b/docs/zh/investigation-edge-display-and-playback.md @@ -0,0 +1,96 @@ +# 节点图边显示异常 & 播放问题排查进展(2026-08-02) + +本文记录当前排查状态,供手工继续排查。随调查更新。 + +## 当前未解决的两个现象 + +### A. 节点图边显示随机缺失 +- 每次显示都不一样:有时全部显示,有时缺几条,缺的边每次不同,无规律。 +- 手工重连能连上;切换选中素材(时间线点选)再切回来,又随机缺。 +- 已确认**不是只有 footage 相关边**受影响,缺的边类型随机。 + +### B. 播放冻结(部分修复后仍有残留报告) +- 历史症状:播放头不动、无声音、画面不动。 +- 已修复两个确定的根因(见"已修复"清单 8、9),复测中。 + +## 已验证的事实(不要再重复验证) + +1. **引擎图是稳定的**。用 C ABI 直接加载 `~/Movies/bbb.ove`,12 条边在 + activate project + 多轮 frame request 后全部存活(/tmp/edge2_repro 验证)。 + 边不显示 ≠ 边被引擎删除。 +2. **项目文件内容正常**。bbb.ove 的 12 条连接(见下"图结构")序列化无误, + clip 节点只声明了 9 个输入(无 pos_in/tex_in/volume_in——那些警告见第 6 条)。 +3. **NodeViewContext 的边创建没有走跳过分支**。`OAK_DEBUG_EDGES=1` 运行时, + "no item for" 跳过日志一条都没有 → 每条边都调用了 `add_edge_internal`, + 边对象是创建了的。问题在创建之后:被事件删掉、或绘制/几何异常。 +4. **删边路径只有两条**:`child_input_disconnected`(NODE_INPUT_DISCONNECTED + 事件驱动,`nodeviewcontext.cpp:251`)和 `remove_child`(节点移除时递归删边)。 +5. **XML/context 解析已排除**。C ABI 实测:引擎解析出的 context 成员与 + bbb.ove 完全一致(sequence=[sequence,track,track]; + clip1=[transform,clip1,footage];clip2=[footage,volume,clip2]), + 12 条边全部存在。图在引擎里 100% 正确,问题只在 app 的 + NodeViewContext 的 item_map_ 里缺节点 item。 +6. **根因已修复(边显示随机缺失)**:`oakengine_node_output_connection_at_ex` + 和 `oakengine_node_output_connection_at` 把 `output_connections()` 的 + `conn.first`(其实是 source 自身)当成目标节点返回,正确目标是 + `conn.second.node()`。后果:NodeViewContext 的 out-edge 枚举拿到的 + "对端"永远是节点自己,`item_map_` 查到自己 → from==to 静默丢弃 → + out-edge 一条都画不出,只能靠 in-edge 补;哪些边缺取决于节点进入 + context 的顺序(成员顺序+时序)→ 表现为随机缺边。已加回归测试 + (oakengine_node_test.cpp test_edges:at_ex 的目标必须是 LUT 而非 solid)。 + 同路径受益者:timelinewidget multicam、nodeparamview 的删边逻辑。 +7. 日志里的 `Failed to retrieve array size of parameter "pos_in"... in Olive.clip` + 和 `"muted_in" ... in Olive.sequence` 是**显示层的错查**,调用栈: + `NodeParamView::update_element_y`(`app/widget/nodeparamview/nodeparamview.cpp:1174`) + 把每个 item 的输入都拿去对 `contexts_.first()` 做 group resolve——硬编码 + 第一个 context,解析错了节点。此 bug 未修,与边显示的关系未确定。 +7. bbb.ove 图结构(12 条边): + - folder.child_in → footage(33755451136), sequence(33848831232) + - sequence.tex_in ← track(33796807680);sequence.samples_in ← track(33796810368) + - sequence.track_in_0/1 ← 两个 track + - track_v.block_in ← clip(33755511232);track_a.block_in ← clip(33755497792) + - clip1.buffer_in ← transform(33764063616);transform.tex_in ← footage + - clip2.buffer_in ← volume(33795740032);volume.samples_in ← footage + +## 已修复的问题(本批,工作区内未全部提交) + +| # | 问题 | 位置 | 状态 | +|---|------|------|------| +| 1 | get_distance_between_nodes 无递归出口栈溢出 | app/widget/nodeparamview/nodeparamview.cpp | 已提交 a4dfc62f0 | +| 2 | viewerdisplay texture_ 悬垂指针(GL 崩溃) | viewerdisplay.{h,cpp} assign_texture | 已提交 a4dfc62f0 | +| 3 | resignal_requests 遍历中改容器 | engine/render/playbackcache.h | 已提交 a4dfc62f0 | +| 4 | preview request 释放后 ticket 回调 UAF | engine/src/capi/preview.cpp | 已提交 a4dfc62f0 | +| 5 | 播放队列帧时间戳全为 0 → 画面不动 | preview.h/.cpp + viewer.cpp | 已提交 a4dfc62f0 | +| 6 | 浮动"查看器"绑定到序列节点 | mainwindow.cpp open_node_in_viewer | 已提交 a4dfc62f0 | +| 7 | Track 析构 UAF + undo remove_track 丢片段 | engine/node/output/track/track.{h,cpp} | 已提交 30853cbcf | +| 8 | 音频 sample_count API 缺失(无声音根因之一) | preview.h/.cpp 新增 get_audio_sample_count | 未提交 | +| 9 | teardown 系列:~Node 中断边事件打到半死对象 | node.cpp silent disconnect;project.cpp is_being_cleared_;clip.cpp marker disconnect 空指针;projectcopier/previewautocacher 项目死后野指针 | 未提交 | +| 10 | 输入 id memcpy 未 NUL 终止(参数名腐坏) | nodeparamviewitem.cpp:40、nodeparamviewwidgetbridge.cpp:145 | 未提交 | +| 11 | 裸事件订阅析构不退订(拖播放头崩溃) | nodeparamviewkeyframecontrol、nodeparamviewconnectedlabel、export dialog 各加析构退订 | 未提交 | + +测试:`cd cmake-build-debug && ctest -j4` 目前 122/122 通过 +(含新增 preview request 端到端回归:单帧+音频请求、teardown 不崩)。 + +## 仍在工作区里的调试代码(提交前需清理) + +- `OAK_DEBUG_EDGES=1`:nodeundo.cpp(Add/RemoveCommand redo/undo)、 + node.cpp disconnect_edge、nodeviewcontext.cpp(edge added/removed/跳过)。 +- `OAK_DEBUG_INVALID_INPUT=1`:node.cpp report_invalid_input 打调用栈。 +- /tmp 下的复现程序(编译产物在 cmake-build-debug/app/ 下): + - `preview_repro`:C ABI 播放请求全流程(单帧+音频+teardown) + - `edge_repro`:工厂建节点连边 + autocache churn + - `edge2_repro`:加载 bbb.ove 验证 12 条边存活 + +## 下一步排查方向(按优先级) + +1. 用最新构建(含 edge added/removed 日志)跑 `OAK_DEBUG_EDGES=1`, + 对照 12 条边在视图里的 add/remove 次数,找幽灵 remove。 +2. 若证实是 load 线程事件与 GUI 建图交错:检查 EngineEventBridge 的 + 事件入队时序 vs NodeView::set_contexts 的建图时机(重复边/乱序删边)。 +3. 修 `update_element_y` 的 `contexts_.first()` 硬编码(应按 item 所属 context resolve)。 +4. clip 缺少 transform 输入(pos_in 等)导致的参数面板/关键帧视图查询失败 + (bbb.ove 里没有这些输入声明,但 app 代码在查)——需确认 Olive 原版 + ClipBlock 是否有这些输入,是被 R8 弄丢的还是本来就不该查。 +5. 音频残留:`Tried to allocate sample buffer with invalid audio parameters` + 仍在日志出现(audio processor 输出参数 channels=0 → fix_channel_layout + 修正为 2,但上游某处仍用 0 声道创建 buffer)。 diff --git a/engine/include/oakengine/preview.h b/engine/include/oakengine/preview.h index b77d44623..f9f9f525c 100644 --- a/engine/include/oakengine/preview.h +++ b/engine/include/oakengine/preview.h @@ -222,6 +222,10 @@ OAKENGINE_API int oakengine_preview_request_get_frame( OAKENGINE_API int oakengine_preview_request_get_audio_channel_count( const OakEnginePreviewRequest *req); +/** @brief Samples per channel in the result, or 0 if none. */ +OAKENGINE_API int oakengine_preview_request_get_audio_sample_count( + const OakEnginePreviewRequest *req); + /** @brief Sample rate of the audio result, or 0 if none. */ OAKENGINE_API int oakengine_preview_request_get_audio_sample_rate( const OakEnginePreviewRequest *req); diff --git a/engine/node/block/clip/clip.cpp b/engine/node/block/clip/clip.cpp index 83337a8bd..4160afbd2 100644 --- a/engine/node/block/clip/clip.cpp +++ b/engine/node/block/clip/clip.cpp @@ -435,30 +435,36 @@ void ClipBlock::invalidate_cache(const TimeRange &range, const QString &from, viewers.isEmpty() ? nullptr : viewers.first(); if (new_connected_viewer != connected_viewer_) { + TimelineMarkerList *old_markers = + connected_viewer_ ? connected_viewer_->get_markers() : nullptr; + if (old_markers) { + disconnect(old_markers, &TimelineMarkerList::marker_added, this, + &ClipBlock::preview_changed); + disconnect(old_markers, &TimelineMarkerList::marker_removed, + this, &ClipBlock::preview_changed); + disconnect(old_markers, &TimelineMarkerList::marker_modified, + this, &ClipBlock::preview_changed); + } if (connected_viewer_) { - disconnect(connected_viewer_->get_markers(), - &TimelineMarkerList::marker_added, this, - &ClipBlock::preview_changed); - disconnect(connected_viewer_->get_markers(), - &TimelineMarkerList::marker_removed, this, - &ClipBlock::preview_changed); - disconnect(connected_viewer_->get_markers(), - &TimelineMarkerList::marker_modified, this, - &ClipBlock::preview_changed); + disconnect(connected_viewer_, &ViewerOutput::destroyed, this, + nullptr); } connected_viewer_ = new_connected_viewer; + TimelineMarkerList *new_markers = + connected_viewer_ ? connected_viewer_->get_markers() : nullptr; + if (new_markers) { + connect(new_markers, &TimelineMarkerList::marker_added, this, + &ClipBlock::preview_changed); + connect(new_markers, &TimelineMarkerList::marker_removed, this, + &ClipBlock::preview_changed); + connect(new_markers, &TimelineMarkerList::marker_modified, + this, &ClipBlock::preview_changed); + } if (connected_viewer_) { - connect(connected_viewer_->get_markers(), - &TimelineMarkerList::marker_added, this, - &ClipBlock::preview_changed); - connect(connected_viewer_->get_markers(), - &TimelineMarkerList::marker_removed, this, - &ClipBlock::preview_changed); - connect(connected_viewer_->get_markers(), - &TimelineMarkerList::marker_modified, this, - &ClipBlock::preview_changed); + connect(connected_viewer_, &ViewerOutput::destroyed, this, + [this]() { connected_viewer_ = nullptr; }); } } diff --git a/engine/node/block/clip/clip.h b/engine/node/block/clip/clip.h index 4b4c6c8e2..c2ba51aae 100644 --- a/engine/node/block/clip/clip.h +++ b/engine/node/block/clip/clip.h @@ -265,6 +265,10 @@ private: TransitionBlock *in_transition_; TransitionBlock *out_transition_; + // Cleared via the viewer's destroyed() signal (see invalidate_cache); + // during project teardown the viewer can die before this clip, and a + // raw dangling pointer here led to disconnecting signals on a dead + // TimelineMarkerList. ViewerOutput *connected_viewer_; private: diff --git a/engine/node/node.cpp b/engine/node/node.cpp index ff6227621..800219105 100644 --- a/engine/node/node.cpp +++ b/engine/node/node.cpp @@ -21,6 +21,8 @@ #include "node.h" +#include + #include #include #include @@ -216,7 +218,7 @@ void Node::connect_edge(Node *output, const NodeInput &input) } } -void Node::disconnect_edge(Node *output, const NodeInput &input) +void Node::disconnect_edge(Node *output, const NodeInput &input, bool silent) { // Ensure graph is the same Q_ASSERT(input.node()->parent() == output->parent()); @@ -232,6 +234,18 @@ void Node::disconnect_edge(Node *output, const NodeInput &input) outputs.erase(std::find(outputs.begin(), outputs.end(), std::pair({ output, input }))); + if (silent) { + // Teardown-only mode: just drop the edge from both maps. No events, + // no signals, no invalidation — the whole graph is being destroyed + // and handlers would touch half-destroyed nodes. + return; + } + + if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) { + qWarning("EDGE-DEBUG: disconnect_edge %p -> %p (%s)", (void *)output, + (void *)input.node(), qPrintable(input.input())); + } + // Call internal events input.node()->InputDisconnectedEvent(input.input(), input.element(), output); @@ -1239,6 +1253,14 @@ Node *Node::copy_node_in_graph(Node *node, MultiUndoCommand *command) void Node::send_invalidate_cache(const TimeRange &range, const InvalidateCacheOptions &options) { + // During project teardown, don't propagate invalidation across edges: + // the nodes on the other side may already be half-destroyed. + if (Project *p = project()) { + if (p->is_being_cleared()) { + return; + } + } + for (const OutputConnection &conn : output_connections_) { // Send clear cache signal to the Node const NodeInput &in = conn.second; @@ -1249,6 +1271,12 @@ void Node::send_invalidate_cache(const TimeRange &range, void Node::invalidate_all(const QString &input, int element) { + if (Project *p = project()) { + if (p->is_being_cleared()) { + return; + } + } + invalidate_cache(TimeRange(RATIONAL_MIN, RATIONAL_MAX), input, element); } @@ -1910,6 +1938,18 @@ void Node::report_invalid_input(const char *attempted_action, const QString &id, qWarning() << "Failed to" << attempted_action << "parameter" << id << "element" << element << "in node" << this->id() << "- input doesn't exist"; + + if (qEnvironmentVariableIsSet("OAK_DEBUG_INVALID_INPUT")) { + void *frames[32]; + const int n = backtrace(frames, 32); + char **symbols = backtrace_symbols(frames, n); + if (symbols) { + for (int i = 0; i < n; i++) { + qWarning("INVALID-INPUT-BT: %s", symbols[i]); + } + free(symbols); + } + } } NodeInputImmediate *Node::create_immediate(const QString &input) @@ -2284,15 +2324,20 @@ bool Node::inputs_from(const QString &id, bool recursively) const void Node::disconnect_all() { + // During project teardown, skip the disconnect events/signals: the + // other side's handlers touch this node's members which are already + // destroyed at ~Node time (e.g. caches), which is a use-after-free. + const bool silent = project() && project()->is_being_cleared(); + // Disconnect inputs (copy map since internal map will change as we disconnect) InputConnections copy = input_connections_; for (auto it = copy.cbegin(); it != copy.cend(); it++) { - disconnect_edge(it->second, it->first); + disconnect_edge(it->second, it->first, silent); } while (!output_connections_.empty()) { OutputConnection conn = output_connections_.back(); - disconnect_edge(conn.first, conn.second); + disconnect_edge(conn.first, conn.second, silent); } } diff --git a/engine/node/node.h b/engine/node/node.h index 812d8e308..037ee99c2 100644 --- a/engine/node/node.h +++ b/engine/node/node.h @@ -417,7 +417,8 @@ public: static void connect_edge(Node *output, const NodeInput &input); - static void disconnect_edge(Node *output, const NodeInput &input); + static void disconnect_edge(Node *output, const NodeInput &input, + bool silent = false); void copy_cache_uuids_from(Node *n); diff --git a/engine/node/nodeundo.cpp b/engine/node/nodeundo.cpp index 2f482c6ca..fd5e8cf40 100644 --- a/engine/node/nodeundo.cpp +++ b/engine/node/nodeundo.cpp @@ -134,6 +134,12 @@ NodeEdgeAddCommand::~NodeEdgeAddCommand() void NodeEdgeAddCommand::redo() { + if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) { + qWarning("EDGE-DEBUG: NodeEdgeAddCommand::redo %p -> %p (%s)", + (void *)output_, (void *)input_.node(), + qPrintable(input_.input())); + } + if (input_.is_connected()) { if (!remove_command_) { remove_command_ = @@ -148,6 +154,12 @@ void NodeEdgeAddCommand::redo() void NodeEdgeAddCommand::undo() { + if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) { + qWarning("EDGE-DEBUG: NodeEdgeAddCommand::undo %p -> %p (%s)", + (void *)output_, (void *)input_.node(), + qPrintable(input_.input())); + } + Node::disconnect_edge(output_, input_); if (remove_command_) { @@ -169,11 +181,23 @@ NodeEdgeRemoveCommand::NodeEdgeRemoveCommand(Node *output, void NodeEdgeRemoveCommand::redo() { + if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) { + qWarning("EDGE-DEBUG: NodeEdgeRemoveCommand::redo %p -> %p (%s)", + (void *)output_, (void *)input_.node(), + qPrintable(input_.input())); + } + Node::disconnect_edge(output_, input_); } void NodeEdgeRemoveCommand::undo() { + if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) { + qWarning("EDGE-DEBUG: NodeEdgeRemoveCommand::undo %p -> %p (%s)", + (void *)output_, (void *)input_.node(), + qPrintable(input_.input())); + } + Node::connect_edge(output_, input_); } diff --git a/engine/node/project.cpp b/engine/node/project.cpp index 770ca20b8..fee87864b 100644 --- a/engine/node/project.cpp +++ b/engine/node/project.cpp @@ -86,6 +86,23 @@ void Project::initialize() void Project::clear() { + is_being_cleared_ = true; + + // Notify observers about each node while it is still fully constructed. + // The childEvent() removal path fires from ~Node, when derived-class + // members are already destroyed; observers touching the node there + // (e.g. querying a Sequence's tracks) used to crash. + const QVector notify_copy = node_children_; + for (Node *node : notify_copy) { + if (!node_children_.contains(node)) { + // An observer already removed it from the graph + continue; + } + emit node_removed(node); + emit node->removed_from_graph(this); + node->RemovedFromGraphEvent(this); + } + // By deleting the last nodes first, we assume that nodes that are most important are deleted last // (e.g. Project's ColorManager or ProjectSettingsNode. for (auto it = node_children_.cbegin(); it != node_children_.cend(); it++) { @@ -95,6 +112,8 @@ void Project::clear() while (!node_children_.isEmpty()) { delete node_children_.last(); } + + is_being_cleared_ = false; } SerializedData Project::load(QXmlStreamReader *reader) @@ -369,6 +388,14 @@ void Project::childEvent(QChildEvent *event) } else if (event->type() == QEvent::ChildRemoved) { node_children_.removeOne(node); + if (is_being_cleared_) { + // Teardown: everything is being destroyed anyway. The + // disconnect/emit dance below touches the half-destroyed + // child (this fires from ~Node) and has repeatedly crashed; + // Qt auto-disconnects the rest when destruction completes. + return; + } + // Disconnect signals disconnect(node, &Node::input_connected, this, &Project::input_connected); diff --git a/engine/node/project.h b/engine/node/project.h index 3a9567e1f..3b63b1cb3 100644 --- a/engine/node/project.h +++ b/engine/node/project.h @@ -103,6 +103,18 @@ public: } void set_modified(bool e); + /** + * @brief True while clear()/teardown is deleting all nodes. + * + * Nodes use this to suppress cache invalidation storms while the graph + * is being torn down: invalidating during destruction walks edges into + * half-destroyed nodes and has repeatedly caused use-after-free crashes. + */ + bool is_being_cleared() const + { + return is_being_cleared_; + } + bool has_autorecovery_been_saved() const; void set_autorecovery_saved(bool e); @@ -259,6 +271,8 @@ private: bool autorecovery_saved_; + bool is_being_cleared_ = false; + ColorManager *color_manager_; QVector node_children_; diff --git a/engine/render/previewautocacher.cpp b/engine/render/previewautocacher.cpp index e0e8f2604..01ccfab52 100644 --- a/engine/render/previewautocacher.cpp +++ b/engine/render/previewautocacher.cpp @@ -855,6 +855,25 @@ void PreviewAutoCacher::set_project(Project *project) project_ = project; if (project_) { + // If the project dies while we're still using it (e.g. shutdown order: + // project freed before the RenderManager), drop all state that + // references its nodes/caches without touching them. Otherwise the + // next set_project(nullptr) would disconnect signals on dead caches. + connect(project_, &Project::destroyed, this, [this]() { + project_ = nullptr; + delayed_requeue_timer_.stop(); + single_frame_render_ = nullptr; + video_immediate_passthroughs_.clear(); + pending_video_jobs_.clear(); + pending_audio_jobs_.clear(); + video_cache_data_.clear(); + audio_cache_data_.clear(); + multicam_ = nullptr; + // The copier's own destroyed-guard nulls its original_; this just + // clears its copy maps without touching the dead project. + copier_->set_project(nullptr); + }); + // Copy graph (this should always be a Project) set_renders_paused(true); diff --git a/engine/render/projectcopier.cpp b/engine/render/projectcopier.cpp index 80ae2b7b5..2fed8e744 100644 --- a/engine/render/projectcopier.cpp +++ b/engine/render/projectcopier.cpp @@ -63,6 +63,11 @@ void ProjectCopier::set_project(Project *project) original_ = project; if (original_) { + // Don't keep a dangling pointer if the project dies while we're + // still around (e.g. RenderManager outlives the project at shutdown) + connect(original_, &Project::destroyed, this, + [this]() { original_ = nullptr; }); + // The copied project is only used as an in-memory render proxy. Mark it so // downstream code (e.g. RenderWorkerPool) knows it is safe to reset its // modified flag after serializing a snapshot. diff --git a/engine/src/capi/node.cpp b/engine/src/capi/node.cpp index 2ee11f5e5..b6237026f 100644 --- a/engine/src/capi/node.cpp +++ b/engine/src/capi/node.cpp @@ -5172,9 +5172,10 @@ int oakengine_node_output_connection_at( return OAKENGINE_E_NOT_FOUND; } const auto &conn = conns[size_t(index)]; - // conn.first = input Node*, conn.second = NodeInput on that node + // output_connections() stores {source (== self), NodeInput}; the + // connection's destination is the NodeInput's node, NOT conn.first. if (input_node) { - *input_node = wrap(conn.first); + *input_node = wrap(conn.second.node()); } if (input_id_buf && input_id_size > 0) { string_to_buf(conn.second.input(), input_id_buf, input_id_size); @@ -5197,9 +5198,10 @@ int oakengine_node_output_connection_at_ex( return OAKENGINE_E_NOT_FOUND; } const auto &conn = conns[size_t(index)]; - // conn.first = input Node*, conn.second = NodeInput on that node + // output_connections() stores {source (== self), NodeInput}; the + // connection's destination is the NodeInput's node, NOT conn.first. if (input_node) { - *input_node = wrap(conn.first); + *input_node = wrap(conn.second.node()); } if (input_id_buf && input_id_size > 0) { string_to_buf(conn.second.input(), input_id_buf, input_id_size); diff --git a/engine/src/capi/preview.cpp b/engine/src/capi/preview.cpp index 7555e40e8..9d6ac878b 100644 --- a/engine/src/capi/preview.cpp +++ b/engine/src/capi/preview.cpp @@ -619,6 +619,25 @@ int oakengine_preview_request_get_audio_channel_count( return s->samples.is_allocated() ? s->samples.channel_count() : 0; } +int oakengine_preview_request_get_audio_sample_count( + const OakEnginePreviewRequest *req) +{ + if (!req) { + return 0; + } + const OakEnginePreviewRequestState *s = + reinterpret_cast(req); + if (!s->ticket || !s->ticket->has_result() || !s->finished->load()) { + return 0; + } + if (!s->has_audio) { + const_cast(s)->samples = + s->ticket->get().value(); + const_cast(s)->has_audio = true; + } + return s->samples.is_allocated() ? int(s->samples.sample_count()) : 0; +} + int oakengine_preview_request_get_audio_sample_rate( const OakEnginePreviewRequest *req) { diff --git a/engine/tests/oakengine_node_test.cpp b/engine/tests/oakengine_node_test.cpp index 657063475..e5a072fe6 100644 --- a/engine/tests/oakengine_node_test.cpp +++ b/engine/tests/oakengine_node_test.cpp @@ -280,6 +280,22 @@ static void test_edges(OakEngineProject *project, OakEngineNode *solid, EXPECT_TRUE(oakengine_node_connect(solid, lut, "tex_in") == OAKENGINE_OK); EXPECT_TRUE(oakengine_node_input_is_connected(lut, "tex_in") == 1); + // Output-side enumeration must report the DESTINATION node (the + // connection's input side), never the source itself — the node view + // builds half its edges through this path. + EXPECT_TRUE(oakengine_node_output_connection_count(solid) > 0); + { + OakEngineNode *dst = NULL; + char id_buf[64]; + int element = -99, hidden = -1; + EXPECT_TRUE(oakengine_node_output_connection_at_ex( + solid, 0, &dst, id_buf, sizeof(id_buf), &element, + &hidden) == OAKENGINE_OK); + EXPECT_TRUE(dst == lut); + EXPECT_TRUE(strcmp(id_buf, "tex_in") == 0); + EXPECT_TRUE(element == -1); + } + // Already-connected input is refused; unknown ids and unconnectable // inputs fail. EXPECT_TRUE(oakengine_node_connect(solid, lut, "tex_in") == diff --git a/engine/tests/oakengine_preview_test.cpp b/engine/tests/oakengine_preview_test.cpp index 61874d49c..f9401064e 100644 --- a/engine/tests/oakengine_preview_test.cpp +++ b/engine/tests/oakengine_preview_test.cpp @@ -36,6 +36,7 @@ #include #endif +#include "oakengine/app.h" #include "oakengine/footage.h" #include "oakengine/init.h" #include "oakengine/preview.h" @@ -44,6 +45,9 @@ #include "oakengine/timeline.h" #include "oakengine/viewer.h" +#include +#include + #ifndef OAK_TEST_SOURCE_DIR #define OAK_TEST_SOURCE_DIR "." #endif @@ -213,6 +217,7 @@ static void test_preview_request_null(void) memset(&frame, 0, sizeof(frame)); EXPECT_TRUE(oakengine_preview_request_get_frame(NULL, &frame) == OAKENGINE_E_INVALID); EXPECT_TRUE(oakengine_preview_request_get_audio_channel_count(NULL) == 0); + EXPECT_TRUE(oakengine_preview_request_get_audio_sample_count(NULL) == 0); EXPECT_TRUE(oakengine_preview_request_get_audio_sample_rate(NULL) == 0); EXPECT_TRUE(oakengine_preview_request_get_audio_samples(NULL, 0, NULL, 0) == OAKENGINE_E_INVALID); oakengine_preview_request_free(NULL); @@ -227,6 +232,61 @@ static void test_render_manager_null(void) EXPECT_TRUE(len >= 0); } +// Drive the request the way ViewerWidget's playback loop does: pump the +// event loop until the ticket finishes (bounded). +static bool pump_until_done(OakEnginePreviewRequest *req, int timeout_ms) +{ + QElapsedTimer t; + t.start(); + while (!oakengine_preview_request_is_done(req) && + !t.hasExpired(timeout_ms)) { + QCoreApplication::processEvents(QEventLoop::AllEvents, 50); + } + return oakengine_preview_request_is_done(req) == 1; +} + +// End-to-end preview request round trip through the RenderManager's worker +// pool: one video frame + one audio range, with the project active. This is +// the exact flow ViewerWidget's playback drives; it also exercises project +// teardown while the RenderManager/copier still reference the project. +static void test_preview_request_roundtrip(OakEngineProject *project, + OakEngineSequence *seq) +{ + EXPECT_TRUE(oakengine_app_set_active_project(project) == OAKENGINE_OK); + + OakEnginePreviewRequest *vreq = + oakengine_preview_request_single_frame((OakEngineNode *)seq, 0, 1, 0); + EXPECT_TRUE(vreq != NULL); + ASSERT_TRUE(pump_until_done(vreq, 60000)); + EXPECT_TRUE(oakengine_preview_request_has_result(vreq) == 1); + oak_playback_frame frame; + memset(&frame, 0, sizeof(frame)); + ASSERT_TRUE(oakengine_preview_request_get_frame(vreq, &frame) == + OAKENGINE_OK); + EXPECT_TRUE(frame.width > 0 && frame.height > 0); + EXPECT_TRUE(frame.data != NULL && frame.linesize > 0); + // The frame must carry a valid timestamp: the viewer's display queue + // orders frames by it (all-zero timestamps froze playback). + EXPECT_TRUE(frame.timestamp_den != 0); + oakengine_preview_request_free(vreq); + + OakEnginePreviewRequest *areq = + oakengine_preview_request_audio_range((OakEngineNode *)seq, 0, 1, 1, 1); + EXPECT_TRUE(areq != NULL); + ASSERT_TRUE(pump_until_done(areq, 60000)); + EXPECT_TRUE(oakengine_preview_request_has_result(areq) == 1); + // The tone clip is stereo 48 kHz, one second long. + EXPECT_TRUE(oakengine_preview_request_get_audio_channel_count(areq) == 2); + EXPECT_TRUE(oakengine_preview_request_get_audio_sample_rate(areq) > 0); + const int sample_count = + oakengine_preview_request_get_audio_sample_count(areq); + EXPECT_TRUE(sample_count > 0); + float buf[48000 * 2]; + EXPECT_TRUE(oakengine_preview_request_get_audio_samples( + areq, 0, buf, qMin(sample_count, 48000 * 2)) > 0); + oakengine_preview_request_free(areq); +} + static void test_playback_cache_null(void) { EXPECT_TRUE(oakengine_viewer_get_playback_cache(NULL) == NULL); @@ -292,10 +352,14 @@ TEST(OakEnginePreview, Main) OAKENGINE_OK); test_levels(seq); test_waveform(tone, demo, probed); + test_preview_request_roundtrip(project, seq); oakengine_footage_free(probed); oakengine_footage_free(demo); oakengine_footage_free(tone); + // Deliberately free the project while it is still the active project: + // the RenderManager/ProjectCopier must survive its destruction, and + // teardown itself must not crash in edge-disconnect invalidation. oakengine_project_free(project); EXPECT_TRUE(oakengine_shutdown() == OAKENGINE_OK);