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

Add AudioListener and PannerNode support #103

Merged
merged 13 commits into from Aug 23, 2018
Merged

Add AudioListener and PannerNode support #103

merged 13 commits into from Aug 23, 2018

Conversation

@Manishearth
Copy link
Member

Manishearth commented Aug 1, 2018

This adds support for AudioListener and AudioPanner

I haven't tested it yet, but it's ready for preliminary review.

@Manishearth Manishearth changed the title [WIP] Add AudioListener and AudioPanner support Add AudioListener and AudioPanner support Aug 4, 2018
@Manishearth
Copy link
Member Author

Manishearth commented Aug 4, 2018

r? @ferjm

@Manishearth
Copy link
Member Author

Manishearth commented Aug 4, 2018

To be clear, I have not run/tested this code at all 😄 , just ensured that it compiles and looks like what the spec says it should.

but it's 5:30 on Friday so I'll get to it next week.

@Manishearth Manishearth changed the title Add AudioListener and AudioPanner support Add AudioListener and PannerNode support Aug 4, 2018

fn get_param(&mut self, id: ParamType) -> &mut Param {
match id {
_ => panic!("Unknown param {:?} for AudioListenerNode", id),

This comment has been minimized.

@Manishearth

Manishearth Aug 4, 2018

Author Member

I should fill this in 😄

@Manishearth Manishearth force-pushed the listener branch 2 times, most recently from f9a119b to 6c165bb Aug 6, 2018
@Manishearth
Copy link
Member Author

Manishearth commented Aug 7, 2018

whoops, forgot to git add the file, so had to squash together all my commits for it.

@Manishearth Manishearth force-pushed the listener branch from 6c165bb to e6a01bd Aug 7, 2018
@Manishearth
Copy link
Member Author

Manishearth commented Aug 7, 2018

Ready for landing cc @ferjm

@Manishearth Manishearth force-pushed the listener branch 3 times, most recently from 67e4774 to 9fccb64 Aug 7, 2018
@ferjm
ferjm approved these changes Aug 23, 2018
Copy link
Member

ferjm left a comment

Nice! r=me with the comments addressed. Thanks!

fn default() -> Self {
PannerNodeOptions {
panning_model: PanningModel::EqualPower,
distance_model: DistanceModel::Linear,

This comment has been minimized.

@ferjm

ferjm Aug 23, 2018

Member

According to the spec this should default to DistanceModel::Inverse.

Self {
channel_info: ChannelInfo {
count: 2,
mode: ChannelCountMode::Max,

This comment has been minimized.

@ferjm

ferjm Aug 23, 2018

Member

This should be ChannelCountMode::ClampledMax

self.orientation_z.update(info, tick)
}

/// Computes azimuthm elevation, and distance of source with respect to a

This comment has been minimized.

@ferjm

ferjm Aug 23, 2018

Member

typo: azimuth, elevation

(azimuth, elevation, distance as f64)
}

fn cone_gain(&self,

This comment has been minimized.

@ferjm

ferjm Aug 23, 2018

Member

It would be nice to add a link to the algorithm spec here


let source_to_listener = normalize_zero(source_position - listener_position);
// Angle between the source orientation vector and the source-listener vector
let angle = 180. * source_to_listener.dot(normalized_source_orientation) / PI;

This comment has been minimized.

@ferjm

ferjm Aug 23, 2018

Member

We are missing an acos here:

let angle = 180. * source_to_listener.dot(normalized_source_orientation).acos() / PI;
}

/// Computes azimuthm elevation, and distance of source with respect to a
/// given AudioListener's position, forward, up vectors

This comment has been minimized.

@ferjm

ferjm Aug 23, 2018

Member

uber-nit: position, forward and up vectors

@Manishearth Manishearth force-pushed the listener branch from 60adc40 to c548060 Aug 23, 2018
@Manishearth Manishearth merged commit a3d2535 into master Aug 23, 2018
@Manishearth
Copy link
Member Author

Manishearth commented Aug 23, 2018

Thanks!

@Manishearth Manishearth deleted the listener branch Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.