fix: memory-safety and playback regressions found via ASan
- nodeparamview: add visited set to get_distance_between_nodes, fixing unbounded recursion (stack overflow) when the node graph has a cycle - viewerdisplay: give texture_ a consistent owner via assign_texture(); borrowed queue textures are now retained, created ones freed, fixing a dangling pointer that corrupted the heap and crashed in the GL driver - playbackcache: resignal_requests() iterates a copy, handlers may clear_request_range() while iterating (ASan container-overflow) - preview C API: preview request ticket lambdas captured the request state raw; after oakengine_preview_request_free the ticket outlived the request and the finished callback wrote into freed memory (heap-use-after-free). The finished flag is now a shared_ptr captured weakly by the callbacks - playback: oak_playback_frame regains a timestamp (num/den) filled from olive::Frame; the viewer queue append no longer uses Rational() for every frame, which made append_timewise drop all but the first frame and froze the picture during playback - mainwindow: open_node_in_viewer refuses sequence nodes; sequences already have the Sequence Viewer, and saved layouts could otherwise resurrect a redundant floating Viewer bound to the sequence
This commit is contained in:
@@ -951,18 +951,25 @@ void NodeParamView::add_node(OakEngineNode *n, OakEngineNode *ctx, NodeParamView
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
int get_distance_between_nodes(OakEngineNode *start, OakEngineNode *end)
|
int get_distance_between_nodes(OakEngineNode *start, OakEngineNode *end,
|
||||||
|
QSet<OakEngineNode *> *visited)
|
||||||
{
|
{
|
||||||
if (start == end) {
|
if (start == end) {
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (visited->contains(start)) {
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
visited->insert(start);
|
||||||
|
|
||||||
const oak::Node start_node(start);
|
const oak::Node start_node(start);
|
||||||
const int connection_count = start_node.input_connection_count_all();
|
const int connection_count = start_node.input_connection_count_all();
|
||||||
for (int i = 0; i < connection_count; i++) {
|
for (int i = 0; i < connection_count; i++) {
|
||||||
OakEngineNode *upstream =
|
OakEngineNode *upstream =
|
||||||
start_node.input_connection_at_all(i).node.handle();
|
start_node.input_connection_at_all(i).node.handle();
|
||||||
int this_node_dist = get_distance_between_nodes(upstream, end);
|
int this_node_dist =
|
||||||
|
get_distance_between_nodes(upstream, end, visited);
|
||||||
if (this_node_dist != -1) {
|
if (this_node_dist != -1) {
|
||||||
return 1 + this_node_dist;
|
return 1 + this_node_dist;
|
||||||
}
|
}
|
||||||
@@ -971,6 +978,12 @@ int get_distance_between_nodes(OakEngineNode *start, OakEngineNode *end)
|
|||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
int get_distance_between_nodes(OakEngineNode *start, OakEngineNode *end)
|
||||||
|
{
|
||||||
|
QSet<OakEngineNode *> visited;
|
||||||
|
return get_distance_between_nodes(start, end, &visited);
|
||||||
|
}
|
||||||
|
|
||||||
void NodeParamView::sort_items_in_context(NodeParamViewContext *context_item)
|
void NodeParamView::sort_items_in_context(NodeParamViewContext *context_item)
|
||||||
{
|
{
|
||||||
QVector<QPair<NodeParamViewItem *, int>> distances;
|
QVector<QPair<NodeParamViewItem *, int>> distances;
|
||||||
|
|||||||
@@ -1836,7 +1836,9 @@ void ViewerWidget::renderer_generated_frame_for_queue()
|
|||||||
QVariant frame = QVariant::fromValue(f);
|
QVariant frame = QVariant::fromValue(f);
|
||||||
|
|
||||||
foreach (ViewerDisplayWidget *dw, playback_devices_) {
|
foreach (ViewerDisplayWidget *dw, playback_devices_) {
|
||||||
dw->queue()->append_timewise({ Rational(), frame },
|
dw->queue()->append_timewise({ Rational(pf.timestamp_num,
|
||||||
|
pf.timestamp_den),
|
||||||
|
frame },
|
||||||
playback_speed_);
|
playback_speed_);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -108,6 +108,18 @@ ViewerDisplayWidget::~ViewerDisplayWidget()
|
|||||||
MANAGEDDISPLAYWIDGET_DEFAULT_DESTRUCTOR_INNER;
|
MANAGEDDISPLAYWIDGET_DEFAULT_DESTRUCTOR_INNER;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void ViewerDisplayWidget::assign_texture(void *t, bool owned)
|
||||||
|
{
|
||||||
|
if (owned_texture_) {
|
||||||
|
oakengine_display_texture_free(owned_texture_);
|
||||||
|
}
|
||||||
|
if (t && !owned) {
|
||||||
|
oakengine_display_texture_retain(t);
|
||||||
|
}
|
||||||
|
owned_texture_ = t;
|
||||||
|
texture_ = t;
|
||||||
|
}
|
||||||
|
|
||||||
void ViewerDisplayWidget::set_matrix_translate(const QMatrix4x4 &mat)
|
void ViewerDisplayWidget::set_matrix_translate(const QMatrix4x4 &mat)
|
||||||
{
|
{
|
||||||
translate_matrix_ = mat;
|
translate_matrix_ = mat;
|
||||||
@@ -483,10 +495,11 @@ void ViewerDisplayWidget::on_paint()
|
|||||||
oakengine_display_texture_height(texture_) != frame_vp.height ||
|
oakengine_display_texture_height(texture_) != frame_vp.height ||
|
||||||
oakengine_display_texture_format(texture_) != frame_vp.format ||
|
oakengine_display_texture_format(texture_) != frame_vp.format ||
|
||||||
oakengine_display_texture_channel_count(texture_) != 4)) {
|
oakengine_display_texture_channel_count(texture_) != 4)) {
|
||||||
texture_ = oakengine_display_texture_create(
|
assign_texture(oakengine_display_texture_create(
|
||||||
renderer(), &frame_vp,
|
renderer(), &frame_vp,
|
||||||
oakengine_codec_frame_data(load_handle),
|
oakengine_codec_frame_data(load_handle),
|
||||||
oakengine_codec_frame_linesize(load_handle));
|
oakengine_codec_frame_linesize(load_handle)),
|
||||||
|
true);
|
||||||
} else if (!drew_backend_neutral_frame) {
|
} else if (!drew_backend_neutral_frame) {
|
||||||
oakengine_display_texture_upload(
|
oakengine_display_texture_upload(
|
||||||
texture_, oakengine_codec_frame_data(load_handle),
|
texture_, oakengine_codec_frame_data(load_handle),
|
||||||
@@ -499,7 +512,7 @@ void ViewerDisplayWidget::on_paint()
|
|||||||
src_ren && src_ren != renderer()) {
|
src_ren && src_ren != renderer()) {
|
||||||
if (oakengine_display_renderer_is_open_gl(src_ren) &&
|
if (oakengine_display_renderer_is_open_gl(src_ren) &&
|
||||||
oakengine_display_renderer_is_open_gl(renderer())) {
|
oakengine_display_renderer_is_open_gl(renderer())) {
|
||||||
texture_ = load_handle;
|
assign_texture(load_handle, false);
|
||||||
} else {
|
} else {
|
||||||
// Cross-backend: download and re-upload
|
// Cross-backend: download and re-upload
|
||||||
void *tmp_frame = oakengine_codec_frame_create();
|
void *tmp_frame = oakengine_codec_frame_create();
|
||||||
@@ -513,24 +526,25 @@ void ViewerDisplayWidget::on_paint()
|
|||||||
&tex_params,
|
&tex_params,
|
||||||
oakengine_codec_frame_data(tmp_frame),
|
oakengine_codec_frame_data(tmp_frame),
|
||||||
oakengine_codec_frame_linesize(tmp_frame));
|
oakengine_codec_frame_linesize(tmp_frame));
|
||||||
texture_ = oakengine_display_texture_create(
|
assign_texture(oakengine_display_texture_create(
|
||||||
renderer(), &tex_params,
|
renderer(), &tex_params,
|
||||||
oakengine_codec_frame_data(tmp_frame),
|
oakengine_codec_frame_data(tmp_frame),
|
||||||
oakengine_codec_frame_linesize(tmp_frame));
|
oakengine_codec_frame_linesize(tmp_frame)),
|
||||||
|
true);
|
||||||
} else {
|
} else {
|
||||||
texture_ = load_handle;
|
assign_texture(load_handle, false);
|
||||||
}
|
}
|
||||||
oakengine_codec_frame_free(tmp_frame);
|
oakengine_codec_frame_free(tmp_frame);
|
||||||
}
|
}
|
||||||
} else if (!drew_backend_neutral_frame) {
|
} else if (!drew_backend_neutral_frame) {
|
||||||
texture_ = load_handle;
|
assign_texture(load_handle, false);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
texture_ = load_custom_texture_from_frame(load_frame_);
|
assign_texture(load_custom_texture_from_frame(load_frame_), true);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (drew_backend_neutral_frame) {
|
if (drew_backend_neutral_frame) {
|
||||||
texture_ = nullptr;
|
assign_texture(nullptr, true);
|
||||||
}
|
}
|
||||||
|
|
||||||
emit texture_changed(texture_);
|
emit texture_changed(texture_);
|
||||||
@@ -762,7 +776,7 @@ void ViewerDisplayWidget::on_destroy()
|
|||||||
|
|
||||||
super::on_destroy();
|
super::on_destroy();
|
||||||
|
|
||||||
texture_ = nullptr;
|
assign_texture(nullptr, true);
|
||||||
if (deinterlace_texture_) {
|
if (deinterlace_texture_) {
|
||||||
oakengine_display_texture_free(deinterlace_texture_);
|
oakengine_display_texture_free(deinterlace_texture_);
|
||||||
deinterlace_texture_ = nullptr;
|
deinterlace_texture_ = nullptr;
|
||||||
|
|||||||
@@ -338,9 +338,29 @@ private:
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* @brief Internal reference to the OpenGL texture to draw. Set in SetTexture() and used in paintGL().
|
* @brief Internal reference to the OpenGL texture to draw. Set in SetTexture() and used in paintGL().
|
||||||
|
*
|
||||||
|
* Never assign directly; use assign_texture() so ownership stays consistent.
|
||||||
*/
|
*/
|
||||||
void *texture_;
|
void *texture_;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Owned (retained or created) reference backing texture_.
|
||||||
|
*
|
||||||
|
* texture_ may point to a texture owned by the playback queue's
|
||||||
|
* OakSharedBuffer; this member holds our own reference so texture_ can
|
||||||
|
* never dangle after the queue entry is replaced.
|
||||||
|
*/
|
||||||
|
void *owned_texture_ = nullptr;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Assign texture_ with consistent ownership.
|
||||||
|
*
|
||||||
|
* Releases the previous owned reference. If owned is true, takes over the
|
||||||
|
* passed reference (e.g. freshly created); otherwise retains it. texture_
|
||||||
|
* is set to the same pointer (possibly nullptr).
|
||||||
|
*/
|
||||||
|
void assign_texture(void *t, bool owned);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @brief Internal texture to deinterlace to
|
* @brief Internal texture to deinterlace to
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -318,6 +318,14 @@ void MainWindow::open_folder(OakEngineNode *i, bool floating)
|
|||||||
|
|
||||||
void MainWindow::open_node_in_viewer(OakEngineNode *node)
|
void MainWindow::open_node_in_viewer(OakEngineNode *node)
|
||||||
{
|
{
|
||||||
|
// Sequences already have the dedicated Sequence Viewer. Opening a
|
||||||
|
// floating Viewer on a Sequence just duplicates it and looks like the
|
||||||
|
// two viewers' controls are swapped. This also filters out stale
|
||||||
|
// <viewer> entries pointing at sequences in saved project layouts.
|
||||||
|
if (!node || oak::Node(node).is_sequence()) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
ViewerPanel *existing = nullptr;
|
ViewerPanel *existing = nullptr;
|
||||||
|
|
||||||
for (auto it = viewer_panels_.cbegin(); it != viewer_panels_.cend(); it++) {
|
for (auto it = viewer_panels_.cbegin(); it != viewer_panels_.cend(); it++) {
|
||||||
|
|||||||
@@ -78,6 +78,8 @@ typedef struct oak_playback_frame {
|
|||||||
int format; /**< olive::PixelFormat::Format value. */
|
int format; /**< olive::PixelFormat::Format value. */
|
||||||
const void *data; /**< Planar data pointer (first plane). */
|
const void *data; /**< Planar data pointer (first plane). */
|
||||||
int linesize; /**< Bytes per row of the first plane. */
|
int linesize; /**< Bytes per row of the first plane. */
|
||||||
|
int64_t timestamp_num; /**< Frame timestamp (seconds) as a rational. */
|
||||||
|
int64_t timestamp_den; /**< 0 when the frame carries no timestamp. */
|
||||||
} oak_playback_frame;
|
} oak_playback_frame;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -131,7 +131,10 @@ public:
|
|||||||
|
|
||||||
void resignal_requests()
|
void resignal_requests()
|
||||||
{
|
{
|
||||||
for (const TimeRange &r : requested_) {
|
// Iterate over a copy: handlers may call clear_request_range() and
|
||||||
|
// mutate requested_ while we're iterating it.
|
||||||
|
const TimeRangeList requests = requested_;
|
||||||
|
for (const TimeRange &r : requests) {
|
||||||
emit requested(request_context_, r);
|
emit requested(request_context_, r);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+40
-12
@@ -429,7 +429,12 @@ int oakengine_preview_cacher_force_cache_range(OakEngineNode *node,
|
|||||||
|
|
||||||
struct OakEnginePreviewRequestState {
|
struct OakEnginePreviewRequestState {
|
||||||
olive::RenderTicketPtr ticket;
|
olive::RenderTicketPtr ticket;
|
||||||
std::atomic<bool> finished{ false };
|
// Shared finished flag. Ticket completion lambdas capture a weak_ptr to
|
||||||
|
// this flag so that freeing the request (oakengine_preview_request_free)
|
||||||
|
// detaches them instead of leaving dangling writes/callbacks behind —
|
||||||
|
// the ticket can outlive the request inside the RenderManager.
|
||||||
|
std::shared_ptr<std::atomic<bool>> finished =
|
||||||
|
std::make_shared<std::atomic<bool>>(false);
|
||||||
bool has_frame = false;
|
bool has_frame = false;
|
||||||
bool has_audio = false;
|
bool has_audio = false;
|
||||||
// Video result
|
// Video result
|
||||||
@@ -460,8 +465,13 @@ oakengine_preview_request_single_frame(OakEngineNode *viewer, int64_t num,
|
|||||||
s->ticket = olive::RenderManager::instance()->get_cacher()->get_single_frame(
|
s->ticket = olive::RenderManager::instance()->get_cacher()->get_single_frame(
|
||||||
v, olive::Rational(num, den), dry != 0);
|
v, olive::Rational(num, den), dry != 0);
|
||||||
if (s->ticket) {
|
if (s->ticket) {
|
||||||
QObject::connect(s->ticket.get(), &olive::RenderTicket::finished,
|
QObject::connect(
|
||||||
[s]() { s->finished.store(true); });
|
s->ticket.get(), &olive::RenderTicket::finished,
|
||||||
|
[weak = std::weak_ptr<std::atomic<bool>>(s->finished)] {
|
||||||
|
if (auto f = weak.lock()) {
|
||||||
|
f->store(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
return reinterpret_cast<OakEnginePreviewRequest *>(s);
|
return reinterpret_cast<OakEnginePreviewRequest *>(s);
|
||||||
}
|
}
|
||||||
@@ -487,8 +497,13 @@ oakengine_preview_request_audio_range(OakEngineNode *viewer, int64_t in_num,
|
|||||||
v, olive::TimeRange(olive::Rational(in_num, in_den),
|
v, olive::TimeRange(olive::Rational(in_num, in_den),
|
||||||
olive::Rational(out_num, out_den)));
|
olive::Rational(out_num, out_den)));
|
||||||
if (s->ticket) {
|
if (s->ticket) {
|
||||||
QObject::connect(s->ticket.get(), &olive::RenderTicket::finished,
|
QObject::connect(
|
||||||
[s]() { s->finished.store(true); });
|
s->ticket.get(), &olive::RenderTicket::finished,
|
||||||
|
[weak = std::weak_ptr<std::atomic<bool>>(s->finished)] {
|
||||||
|
if (auto f = weak.lock()) {
|
||||||
|
f->store(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
return reinterpret_cast<OakEnginePreviewRequest *>(s);
|
return reinterpret_cast<OakEnginePreviewRequest *>(s);
|
||||||
}
|
}
|
||||||
@@ -500,7 +515,7 @@ int oakengine_preview_request_is_done(const OakEnginePreviewRequest *req)
|
|||||||
}
|
}
|
||||||
const OakEnginePreviewRequestState *s =
|
const OakEnginePreviewRequestState *s =
|
||||||
reinterpret_cast<const OakEnginePreviewRequestState *>(req);
|
reinterpret_cast<const OakEnginePreviewRequestState *>(req);
|
||||||
return s->ticket && s->finished.load() ? 1 : 0;
|
return s->ticket && s->finished->load() ? 1 : 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
int oakengine_preview_request_has_result(const OakEnginePreviewRequest *req)
|
int oakengine_preview_request_has_result(const OakEnginePreviewRequest *req)
|
||||||
@@ -524,10 +539,18 @@ int oakengine_preview_request_set_finished_callback(
|
|||||||
if (!s->ticket) {
|
if (!s->ticket) {
|
||||||
return OAKENGINE_E_INVALID;
|
return OAKENGINE_E_INVALID;
|
||||||
}
|
}
|
||||||
// Connect the ticket's finished signal to call the callback.
|
// Connect the ticket's finished signal to call the callback. Guarded by
|
||||||
|
// the request's shared flag so a freed request suppresses the callback.
|
||||||
if (callback) {
|
if (callback) {
|
||||||
QObject::connect(s->ticket.get(), &olive::RenderTicket::finished,
|
QObject::connect(
|
||||||
[callback, user_data]() { callback(user_data); });
|
s->ticket.get(), &olive::RenderTicket::finished,
|
||||||
|
[callback, user_data,
|
||||||
|
weak = std::weak_ptr<std::atomic<bool>>(s->finished)] {
|
||||||
|
if (auto f = weak.lock()) {
|
||||||
|
f->store(true);
|
||||||
|
callback(user_data);
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
return OAKENGINE_OK;
|
return OAKENGINE_OK;
|
||||||
}
|
}
|
||||||
@@ -544,7 +567,7 @@ int oakengine_preview_request_get_frame(OakEnginePreviewRequest *req,
|
|||||||
return OAKENGINE_E_INVALID;
|
return OAKENGINE_E_INVALID;
|
||||||
}
|
}
|
||||||
// Wait for finish if not done (pump events).
|
// Wait for finish if not done (pump events).
|
||||||
if (!s->finished.load()) {
|
if (!s->finished->load()) {
|
||||||
QCoreApplication::processEvents();
|
QCoreApplication::processEvents();
|
||||||
return OAKENGINE_E_INVALID;
|
return OAKENGINE_E_INVALID;
|
||||||
}
|
}
|
||||||
@@ -567,6 +590,11 @@ int oakengine_preview_request_get_frame(OakEnginePreviewRequest *req,
|
|||||||
out->height = s->frame_height;
|
out->height = s->frame_height;
|
||||||
out->format = s->frame_format;
|
out->format = s->frame_format;
|
||||||
out->data = s->frame->data();
|
out->data = s->frame->data();
|
||||||
|
{
|
||||||
|
const olive::Rational ts = s->frame->timestamp();
|
||||||
|
out->timestamp_num = ts.numerator();
|
||||||
|
out->timestamp_den = ts.denominator();
|
||||||
|
}
|
||||||
out->linesize = s->frame->linesize_bytes();
|
out->linesize = s->frame->linesize_bytes();
|
||||||
return OAKENGINE_OK;
|
return OAKENGINE_OK;
|
||||||
}
|
}
|
||||||
@@ -579,7 +607,7 @@ int oakengine_preview_request_get_audio_channel_count(
|
|||||||
}
|
}
|
||||||
const OakEnginePreviewRequestState *s =
|
const OakEnginePreviewRequestState *s =
|
||||||
reinterpret_cast<const OakEnginePreviewRequestState *>(req);
|
reinterpret_cast<const OakEnginePreviewRequestState *>(req);
|
||||||
if (!s->ticket || !s->ticket->has_result() || !s->finished.load()) {
|
if (!s->ticket || !s->ticket->has_result() || !s->finished->load()) {
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
if (!s->has_audio) {
|
if (!s->has_audio) {
|
||||||
@@ -614,7 +642,7 @@ int oakengine_preview_request_get_audio_samples(
|
|||||||
}
|
}
|
||||||
OakEnginePreviewRequestState *s =
|
OakEnginePreviewRequestState *s =
|
||||||
reinterpret_cast<OakEnginePreviewRequestState *>(req);
|
reinterpret_cast<OakEnginePreviewRequestState *>(req);
|
||||||
if (!s->ticket || !s->ticket->has_result() || !s->finished.load()) {
|
if (!s->ticket || !s->ticket->has_result() || !s->finished->load()) {
|
||||||
return OAKENGINE_E_INVALID;
|
return OAKENGINE_E_INVALID;
|
||||||
}
|
}
|
||||||
if (!s->has_audio) {
|
if (!s->has_audio) {
|
||||||
|
|||||||
Reference in New Issue
Block a user