Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic graph abstraction #45

Merged
merged 4 commits into from Jun 7, 2018

Conversation

Projects
None yet
2 participants
@Manishearth
Copy link
Member

commented Jun 6, 2018

Todo: Using it in the code

r? @ferjm

@Manishearth

This comment has been minimized.

Copy link
Member Author

commented Jun 6, 2018

Can be merged as-is, opening it early so I can get feedback on the design before i integrate it with everything else.

@ferjm

ferjm approved these changes Jun 7, 2018

Copy link
Member

left a comment

Looks good so far! Great work as usual :)

I have a couple of questions:

  • What's the plan for cycles?
  • What's the plan for connections between AudioNodes and AudioParams?
self.dest_id
}

pub fn process(&mut self, info: &BlockInfo) -> Chunk {

This comment has been minimized.

Copy link
@ferjm

ferjm Jun 7, 2018

Member

This method could use some comments ;)

}

pub struct Node {
node: RefCell<Box<AudioNodeEngine>>,

This comment has been minimized.

Copy link
@ferjm

ferjm Jun 7, 2018

Member

Note that AudioParams may be graph nodes as well.

fn output_count(&self) -> u32 { 1 }

/// If we're the destination node, extract the contained data
fn destination_data(&mut self) -> Option<Chunk> { None }

This comment has been minimized.

Copy link
@ferjm

ferjm Jun 7, 2018

Member

Could we reuse process for this? With an extra flag in BlockInfo probably.

This comment has been minimized.

Copy link
@Manishearth

Manishearth Jun 7, 2018

Author Member

How? BlockInfo is immutable (and not worth a refcell IMO)

I don't want to do the model of the destination node pushing directly to the sink because the spec represents this as a separate "record the input for future retrieval" step. But we can still do that for now if you think it's better.

This comment has been minimized.

Copy link
@ferjm

ferjm Jun 7, 2018

Member

Ok. I guess this is fine then.

I reverted 6131d39

@Manishearth

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2018

For cycles: we can do the traversal a little bit more manually to support it, the algorithm isn't too hard. I was thinking once this is implemented we can parallelize out that work to support all the little graph things.

@Manishearth

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2018

For AudioParams the plan was to allow for params to "create" new input ports. We can have an additional boolean flag on PortId, and when flipped it enumerates params instead.

@ferjm

This comment has been minimized.

Copy link
Member

commented Jun 7, 2018

I am going to go ahead and merge this as-is, as I am moving lots of code for the AudioBufferSourceNode work and this will likely cause conflicts.

@ferjm ferjm merged commit 3e46719 into master Jun 7, 2018

@ferjm ferjm deleted the graph branch Jun 7, 2018

@Manishearth

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.