fix: production defects and observability found by the test suites

- ForceParams: hand-written Default with force_format = -1 (was 0 = U8,
  which pushed the F32 pipeline into the U8 scale path).
- Plugin clip output: write CPU pixels back into the target texture
  instead of the deep clone returned by texture_get_frame.
- Display ICC: probe the Debian/Ubuntu icc-profiles-free path.
- RippleInfo: public constructor and accessors so the ripple command is
  reachable from integration tests.
- MockEngine: record effect-parameter and push-button attempts so the
  params-view routing tests are falsifiable.
- OFX params: log rejected parameter writes instead of discarding them.
- Manager docs: state the synchronous codec-submission contract.
This commit is contained in:
2026-09-22 20:54:03 +08:00
parent f188bc79e7
commit 18abdec423
6 changed files with 191 additions and 25 deletions
+74
View File
@@ -493,6 +493,17 @@ pub struct MockEngine {
effects: Vec<MockEffect>,
/// Id allocator for effects added at runtime.
next_effect_id: u64,
/// Parameter-write attempts from the params view (`input_id` + value
/// debug), recorded even when the call fails, so the routing tests are
/// falsifiable (review §10.2: previously the mock swallowed every write
/// and the tests could not observe it).
effect_param_attempts: Mutex<Vec<(EffectId, String, String)>>,
/// Push-button attempts from the params view.
effect_push_attempts: Mutex<Vec<(EffectId, String)>>,
/// Result returned by `set_effect_param`/`effect_push_button` (default
/// `Err`: the demo models an unsupported node; tests can flip it to cover
/// the success path).
effect_param_ok: bool,
/// Whether the program monitor is playing (mirrors the clock; kept here
/// because the audio-meter data source has no `App` to read the clock).
program_playing: bool,
@@ -588,6 +599,29 @@ pub struct MockEngine {
impl MockEngine {
/// Builds the demo project: 第一稿.ove, one HD sequence, four tracks.
/// The parameter-write attempts the params view routed to this engine
/// (`(effect, input_id, value debug)`), in call order.
pub fn effect_param_attempts(&self) -> Vec<(EffectId, String, String)> {
self.effect_param_attempts
.lock()
.unwrap_or_else(|e| e.into_inner())
.clone()
}
/// The push-button attempts the params view routed to this engine.
pub fn effect_push_attempts(&self) -> Vec<(EffectId, String)> {
self.effect_push_attempts
.lock()
.unwrap_or_else(|e| e.into_inner())
.clone()
}
/// Flip the result of `set_effect_param`/`effect_push_button` (default
/// `Err`); tests use `true` to cover the accepted path.
pub fn set_effect_param_ok(&mut self, ok: bool) {
self.effect_param_ok = ok;
}
pub fn demo(cx: &mut Context<Self>) -> Self {
let rate = VideoFormat::hd_1080p25().rate;
let clip =
@@ -862,6 +896,9 @@ impl MockEngine {
},
],
next_effect_id: 4,
effect_param_attempts: Mutex::new(Vec::new()),
effect_push_attempts: Mutex::new(Vec::new()),
effect_param_ok: false,
program_playing: false,
selected_item: None,
meter_phase: 0,
@@ -1681,6 +1718,43 @@ impl AppEngine for MockEngine {
})
}
fn set_effect_param(
&mut self,
effect: EffectId,
input_id: &str,
value: oak_node::value::NodeValue,
cx: &mut Context<Self>,
) -> Result<(), String> {
self.effect_param_attempts
.lock()
.unwrap_or_else(|e| e.into_inner())
.push((effect, input_id.to_string(), format!("{value:?}")));
if self.effect_param_ok {
cx.notify();
Ok(())
} else {
Err("the demo engine does not apply effect parameters".into())
}
}
fn effect_push_button(
&mut self,
effect: EffectId,
input_id: &str,
cx: &mut Context<Self>,
) -> Result<(), String> {
self.effect_push_attempts
.lock()
.unwrap_or_else(|e| e.into_inner())
.push((effect, input_id.to_string()));
if self.effect_param_ok {
cx.notify();
Ok(())
} else {
Err("the demo engine does not trigger effect push buttons".into())
}
}
fn add_node_at(
&mut self,
type_id: &str,
+20 -8
View File
@@ -873,7 +873,9 @@ fn wire_controls<E: AppEngine>(view: &OfxParamsView<E>, cx: &mut Context<OfxPara
_ => NodeValue::Float(value.to_f64()),
};
engine.update(cx, |engine, cx| {
let _ = engine.set_effect_param(effect, &input_id, nv, cx);
if let Err(err) = engine.set_effect_param(effect, &input_id, nv, cx) {
eprintln!("[ofx params] set {input_id:?} failed: {err}");
}
});
}
},
@@ -886,7 +888,9 @@ fn wire_controls<E: AppEngine>(view: &OfxParamsView<E>, cx: &mut Context<OfxPara
let CheckBoxEvent::Toggled { state, .. } = event;
let nv = NodeValue::Boolean(*state == CheckState::Checked);
engine.update(cx, |engine, cx| {
let _ = engine.set_effect_param(effect, &input_id, nv, cx);
if let Err(err) = engine.set_effect_param(effect, &input_id, nv, cx) {
eprintln!("[ofx params] set {input_id:?} failed: {err}");
}
});
})
.detach();
@@ -912,7 +916,9 @@ fn wire_controls<E: AppEngine>(view: &OfxParamsView<E>, cx: &mut Context<OfxPara
_ => NodeValue::Combo(*value as i64),
};
engine.update(cx, |engine, cx| {
let _ = engine.set_effect_param(effect, &input_id, nv, cx);
if let Err(err) = engine.set_effect_param(effect, &input_id, nv, cx) {
eprintln!("[ofx params] set {input_id:?} failed: {err}");
}
});
})
.detach();
@@ -1023,12 +1029,14 @@ fn wire_controls<E: AppEngine>(view: &OfxParamsView<E>, cx: &mut Context<OfxPara
.collect();
let json = oak_plugin::param_curve::curves_to_json(&curves);
engine.update(cx, |engine, cx| {
let _ = engine.set_effect_param(
if let Err(err) = engine.set_effect_param(
effect,
&input_id,
NodeValue::Text(json),
cx,
);
) {
eprintln!("[ofx params] set {input_id:?} failed: {err}");
}
});
},
)
@@ -1105,7 +1113,9 @@ impl<E: AppEngine> Render for OfxParamsView<E> {
.child(button_label)
.on_click(move |_event: &ClickEvent, _window, cx| {
engine.update(cx, |engine, cx| {
let _ = engine.effect_push_button(effect, &input_id, cx);
if let Err(err) = engine.effect_push_button(effect, &input_id, cx) {
eprintln!("[ofx params] push {input_id:?} failed: {err}");
}
});
})
.into_any_element()
@@ -1204,12 +1214,14 @@ impl<E: AppEngine> Render for OfxParamsView<E> {
.on_click(move |_event: &ClickEvent, _window, cx| {
let text = editor_commit.read(cx).as_str().to_string();
engine.update(cx, |engine, cx| {
let _ = engine.set_effect_param(
if let Err(err) = engine.set_effect_param(
effect,
&input_id,
NodeValue::Text(text),
cx,
);
) {
eprintln!("[ofx params] set {input_id:?} failed: {err}");
}
});
}),
)
+17 -6
View File
@@ -346,7 +346,7 @@ impl ClipInstance {
use crate::error::Error;
use crate::render::{texture_get_frame, PIXEL_FORMAT_F32};
let texture = self
let mut texture = self
.output_texture
.lock()
.unwrap_or_else(|e| e.into_inner())
@@ -384,11 +384,22 @@ impl ClipInstance {
let s = y * tight;
dst_bytes[d..d + tight].copy_from_slice(&pixels[s..s + tight]);
}
// GPU 目标纹理:拷贝只落在下载帧上,经后端 upload 回写
// (CPU 纹理无需上传)。
if let crate::render::Texture::Gpu { token, ctx, .. } = &texture {
ctx.upload(*token, &frame)
.map_err(|e| Error::Failed(format!("输出纹理上传失败:{e}")))?;
// 写回目标纹理:CPU 纹理的 `to_frame` 是深拷贝,必须把改写
// 后的帧放回本体(同 render_driver::write_output_frame 的值
// 模型修复);GPU 目标纹理:拷贝只落在下载帧上,经后端
// upload 回写。
match &mut texture {
crate::render::Texture::Cpu(f) => {
f.data = frame.data;
}
crate::render::Texture::Gpu { token, ctx, .. } => {
ctx.upload(*token, &frame)
.map_err(|e| Error::Failed(format!("输出纹理上传失败:{e}")))?;
}
// 未解析的平面纹理不会成为插件输出目标(解码路径会先解析)。
crate::render::Texture::Planar(_) => {
return Err(Error::Failed("平面纹理不能作为插件输出目标".into()));
}
}
Ok(texture)
}
+38 -10
View File
@@ -16,9 +16,11 @@
//! The `TaskManager` singleton, mirroring `src/task/src/taskmanager.h`.
//!
//! Holds the set of live [`crate::task::Task`] instances; starting a task
//! hands its ownership to the manager, which also exposes the codec task
//! submitter registration (see [`crate::codecbridge`]).
//! Holds the set of live [`crate::task::Task`] instances and starts them
//! on worker threads. Codec task-submitter registration is a **separate**
//! concern owned by [`crate::codecbridge`]: the embedding application
//! calls `register_codec_task_submitter()` explicitly (the manager never
//! installs the callback itself — see [`TaskManager::init`]).
//!
//! CPP-PARITY: src/task/src/taskmanager.h
//!
@@ -43,7 +45,10 @@ pub struct TaskManager {
tasks: Vec<Box<dyn TaskBox>>,
/// Worker threads, parallel to `tasks` (moved out before joining).
threads: Vec<Option<std::thread::JoinHandle<()>>>,
/// Whether the codec task submitter is currently registered.
/// Whether the embedding application registered the codec task
/// submitter (`crate::codecbridge::register_codec_task_submitter`).
/// Pure bookkeeping on the manager: [`TaskManager::init`] never
/// registers anything itself, so a fresh singleton reads `false`.
codec_submitter_registered: bool,
}
@@ -81,9 +86,23 @@ impl TaskManager {
guard.as_ref().map(|p| unsafe { &*p.0 })
}
/// Create the singleton and register the codec task submitter. Returns
/// `Err(Error::State)` if already initialized (mirrors
/// `oaktask_manager_init`).
/// Create the singleton. Returns `Err(Error::State)` if already
/// initialized (mirrors `oaktask_manager_init`).
///
/// This **does not** register the codec task submitter: registration is
/// `crate::codecbridge::register_codec_task_submitter`, an explicit
/// call by the embedding application. Two reasons:
///
/// - the C++ parity (`TaskManager::create_instance`) creates the
/// manager only; codecbridge wires the callback separately, and
/// - the codec bridge's interim contract is *synchronous* submission
/// (`submit_codec_task` runs the task inline), so registration needs
/// no manager state.
///
/// Consequently a fresh manager reports
/// [`TaskManager::codec_submitter_registered`] as `false`; the
/// application flips it via [`TaskManager::set_codec_submitter_registered`]
/// when it performs the registration itself.
pub fn init() -> Result<()> {
let mut guard = INSTANCE.lock().unwrap();
if guard.is_some() {
@@ -98,7 +117,9 @@ impl TaskManager {
Ok(())
}
/// Destroy the singleton. Idempotent.
/// Destroy the singleton. Idempotent. Does not unregister the codec
/// task submitter (the registration is owned by the embedding
/// application, not by the manager — see [`TaskManager::init`]).
pub fn shutdown() {
let ptr = {
let mut guard = INSTANCE.lock().unwrap();
@@ -231,12 +252,19 @@ impl TaskManager {
out
}
/// Whether this manager registered the codec submitter itself.
/// Whether this manager's bookkeeping flag says the embedding
/// application registered the codec submitter. Never set by
/// [`TaskManager::init`] (which registers nothing); see
/// [`TaskManager::set_codec_submitter_registered`].
pub fn codec_submitter_registered(&self) -> bool {
self.codec_submitter_registered
}
/// Mark whether this manager registered the codec submitter.
/// Record whether the embedding application registered the codec
/// submitter. This is pure bookkeeping: it neither installs nor
/// removes the oakcodec callback (that is
/// `crate::codecbridge::register_codec_task_submitter` /
/// `unregister_codec_task_submitter`).
pub fn set_codec_submitter_registered(&mut self, registered: bool) {
self.codec_submitter_registered = registered;
}
+18 -1
View File
@@ -83,7 +83,7 @@ const TICKET_AUDIO: i32 = ticket_kind::AUDIO;
/// dropped with the C ABI: the direct ticket arena carries a forced size
/// and pixel format only (the eval producer performs no color
/// management). The matrix fields are retained for API parity but unused.
#[derive(Clone, Debug, Default)]
#[derive(Clone, Debug)]
pub struct ForceParams {
/// Forced output width (0 = off).
pub force_width: i32,
@@ -100,6 +100,22 @@ pub struct ForceParams {
pub force_channel_count: i32,
}
impl Default for ForceParams {
/// All overrides off: in particular `force_format` is `-1` ("off"), not
/// `0` (which is a real `PixelFormat` code — U8, and the F32 render
/// pipeline rejects it).
fn default() -> Self {
ForceParams {
force_width: 0,
force_height: 0,
force_matrix: [0.0; 16],
has_force_matrix: false,
force_format: -1,
force_channel_count: 0,
}
}
}
/// Subclass hooks, standing in for the C++ protected virtuals
/// `download_frame`/`frame_downloaded`/`audio_downloaded`/`encode_subtitle`.
/// Frames and audio are the direct `oakrender` value types (the deleted
@@ -1225,3 +1241,4 @@ fn shm_frame_to_texture(frame: &ShmFrameRef) -> oak_core::texture::Texture {
.collect();
oak_core::texture::Texture::wrap_frame(f)
}
.collect();
+24
View File
@@ -580,6 +580,30 @@ pub struct RippleInfo {
append_gap: bool,
}
impl RippleInfo {
/// Creates a per-block ripple record: `block` is the block whose length
/// the ripple changes, and `append_gap` requests inserting a gap of the
/// movement span ahead of it instead (C++
/// `TrackListRippleToolCommand::RippleInfo{block, append_gap}`).
///
/// Added for the M6 test-coverage pass: without a constructor the
/// integration tests could only build an empty info list, leaving the
/// whole `ripple()` body unreachable (review §M5).
pub fn new(block: NodeRef, append_gap: bool) -> Self {
Self { block, append_gap }
}
/// The block this record targets.
pub fn block(&self) -> &NodeRef {
&self.block
}
/// Whether the ripple inserts a gap instead of resizing the block.
pub fn append_gap(&self) -> bool {
self.append_gap
}
}
/// `TrackListRippleToolCommand` — ripple-tool edit: shift blocks on the listed
/// tracks by `ripple_movement` (timelineundoripple.h).
#[allow(dead_code)] // `track_list` mirrors the C++ member; the per-track info drives the work.