Skip to content

/ servo

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

Pass through channel settings in AudioNode constructor #21674

Merged
merged 3 commits into from Sep 13, 2018
Merged

Pass through channel settings in AudioNode constructor #21674

merged 3 commits into from Sep 13, 2018
+197 −178

Conversation

@Manishearth
Copy link
Member

Manishearth commented Sep 11, 2018 •
edited

Most audionodes let you pass in channel count/etc settings in their
constructors, and have different defaults. Using the create_node
argument added in servo/media#124 , this passes
that information through.

r? @ferjm.

This change is Reviewable
@highfive
Copy link

highfive commented Sep 11, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/oscillatornode.rs, components/script/dom/audionode.rs, components/script/dom/audiobuffersourcenode.rs, components/script/dom/audioscheduledsourcenode.rs, components/script/dom/audiodestinationnode.rs and 4 more
  • @KiChjang: components/script/dom/oscillatornode.rs, components/script/dom/audionode.rs, components/script/dom/audiobuffersourcenode.rs, components/script/dom/audioscheduledsourcenode.rs, components/script/dom/audiodestinationnode.rs and 4 more
@Manishearth Manishearth force-pushed the Manishearth:channel-count branch from f3385d4 to 82df7a8 Sep 11, 2018
@Manishearth Manishearth changed the title Pass through channel settings in AudioNode constructor, handle DecodeAudio's channel count Pass through channel settings in AudioNode constructor Sep 11, 2018
@Manishearth
Copy link
Member Author

Manishearth commented Sep 11, 2018

Removed the bits that handle decoding, see #21539
@bors-servo
Copy link
Contributor

bors-servo commented Sep 11, 2018

☔ The latest upstream changes (presumably #21539) made this pull request unmergeable. Please resolve the merge conflicts.
Manishearth added 2 commits Sep 11, 2018
@Manishearth
Update servo-media
9779ce3
@Manishearth
Pass down ChannelInfo to create_node
Loading status checks…
9254606
@ferjm
ferjm approved these changes Sep 11, 2018
View changes
Copy link
Member

ferjm left a comment

Looks good. r=me

Thanks!
@Manishearth Manishearth force-pushed the Manishearth:channel-count branch from 82df7a8 to 9254606 Sep 11, 2018
@jdm
Copy link
Member

jdm commented Sep 11, 2018 

Checking files for tidiness...
./Cargo.lock:1:duplicate versions for package `num-rational`
The following packages depend on version 0.1.42 from 'crates.io':
		image
The following packages depend on version 0.2.1 from 'crates.io':
		gstreamer
./Cargo.lock:1:duplicate versions for package `byte-slice-cast`
The following packages depend on version 0.1.0 from 'crates.io':
		servo-media-audio
The following packages depend on version 0.2.0 from 'crates.io':
		servo-media-gstreamer
./components/script/dom/audiodestinationnode.rs:9:use statement is not in alphabetical order
expected: dom::bindings::codegen::Bindings::AudioNodeBinding::AudioNodeOptions
found: dom::bindings::codegen::Bindings::AudioNodeBinding::{ChannelCountMode, ChannelInterpretation}
@Manishearth
Copy link
Member Author

Manishearth commented Sep 11, 2018

Opened image-rs/image#809 for the image crate, but that may take time, so I'll whitelist it

servo/media#126 for the servo-media one.
@ferjm ferjm mentioned this pull request Sep 12, 2018
Merged
@ferjm
Make tidy happy
Loading status checks…
831201c
@ferjm
Copy link
Member

ferjm commented Sep 12, 2018

@bors-servo try=wpt
@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2018

⌛ Trying commit 831201c with merge 118f245...
bors-servo added a commit that referenced this pull request Sep 12, 2018
@bors-servo
Auto merge of #21674 - Manishearth:channel-count, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
118f245 
Pass through channel settings in AudioNode constructor

Most audionodes let you pass in channel count/etc settings in their
constructors, and have different defaults. Using the `create_node`
argument added in servo/media#124 , this passes
that information through.

r? @ferjm.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21674)
<!-- Reviewable:end -->
@ferjm ferjm removed the S-fails-tidy label Sep 12, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2018

☀️ Test successful - linux-rel-css, linux-rel-wpt
State: approved= try=True
@ferjm
Copy link
Member

ferjm commented Sep 12, 2018

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2018

📌 Commit 831201c has been approved by ferjm
@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2018

⌛ Testing commit 831201c with merge cd02ca6...
bors-servo added a commit that referenced this pull request Sep 12, 2018
@bors-servo
Auto merge of #21674 - Manishearth:channel-count, r=ferjm
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
cd02ca6 
Pass through channel settings in AudioNode constructor

Most audionodes let you pass in channel count/etc settings in their
constructors, and have different defaults. Using the `create_node`
argument added in servo/media#124 , this passes
that information through.

r? @ferjm.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21674)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 13, 2018

☀️ Test successful - android, android-x86, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: ferjm
Pushing cd02ca6 to master...
@bors-servo bors-servo merged commit 831201c into servo:master Sep 13, 2018
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Sep 13, 2018
Merged
3 of 3 tasks complete
@ferjm ferjm added this to Done in WebAudio Nov 29, 2018
@Manishearth Manishearth deleted the Manishearth:channel-count branch May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ferjm ferjm

Assignees

@ferjm ferjm

Labels
None yet
Projects
WebAudio
  
Done
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@Manishearth @highfive @bors-servo @jdm @ferjm
You can’t perform that action at this time.