Skip to content

Make AudioNode::info and AudioNode::construct_processor fallible#117

Open
IRSMsoso wants to merge 7 commits intoBillyDM:mainfrom
IRSMsoso:fallible-node-construction
Open

Make AudioNode::info and AudioNode::construct_processor fallible#117
IRSMsoso wants to merge 7 commits intoBillyDM:mainfrom
IRSMsoso:fallible-node-construction

Conversation

@IRSMsoso
Copy link

These methods being fallible allows for node authors to have conditions for which a node cannot be created without panicking. For example, a ConvolutionNode with greater than 2 channels.

The original motivation for introducing fallibility to these functions was for the implementation of a CLAP plugin node, which can fail instantiation for a multitude of reasons both during the AudioNode::info stage and the AudioNode::construct_processor stage.

@CorvusPrudens suggested using an anyhow-like error type, but wasn't sure how deep those should go before switching to structured thiserror types or simply panicking (which I decided to do for construction of nodes that are currently known to never fail). Suggestions welcome.

@IRSMsoso IRSMsoso marked this pull request as ready for review March 15, 2026 19:35
@IRSMsoso IRSMsoso requested a review from CorvusPrudens March 15, 2026 21:33
@IRSMsoso
Copy link
Author

Hmm, not sure what to do about no_std

Copy link
Contributor

@CorvusPrudens CorvusPrudens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A touch nitpicky, but I think the rest of the changes look good.


#[derive(Error, Debug)]
#[error("ConvolutionNode::CHANNELS cannot be greater than 2, got {0}")]
pub struct ConvolutionInvalidChannelCount(usize);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, let's make sure the field is public!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, you're right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants