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

When splitting channels, allow coloring and filtering of channels #1947

Merged
merged 8 commits into from
May 18, 2020

Conversation

win-chen
Copy link
Contributor

Our project would like to allow the user to filter out channels from the audio for playing/viewing. The filtering for playing will be done via setFilters. This pr is to add the rest via a new parameter splitChannelsOptions and a new method setFilteredChannels.

Short description of changes:

  1. Add splitChannelsOptions parameter that applies if splitChannels is true.
    Contains the following options:
    overlay - draws channels on over each other when splitting
    channelColors - defines colors for channels individually
    filteredChannels - channel indices to filter (do not draw on the waveform)

  2. Add setFilteredChannels method to update the filteredChannels param.

  3. Update split channels example to include info on the above.

Breaking in the external API:

none

Breaking changes in the internal API:

none

Todos/Notes:

The splitChannelsOptions currently only works with drawWave.

Related Issues and other PRs:

coloring:
#1889

@coveralls
Copy link

coveralls commented May 14, 2020

Coverage Status

Coverage decreased (-0.2%) to 80.372% when pulling 69e835c on win-chen:master into 5c6db87 on katspaugh:master.

@thijstriemstra
Copy link
Contributor

this looks similar to #1889, just a different implementation, right @win-chen?

@win-chen
Copy link
Contributor Author

Yea it's similar. The differences are:

#1889 creates two canvases and waveforms in order to color the different waves. This pr adds changes into existing methods to apply colors.

The shape of the parameters are different. That one adds a waveColor2 and progressColor2 so it allows coloring two channels. This one sets colors using a map from channel index to a { waveColor, progressColor } object so it should work with audio that has more channels than stereo.

This one includes hiding different channels and stacking the wave forms on top of each other which are not part of #1889.

src/drawer.multicanvas.js Show resolved Hide resolved
src/drawer.multicanvas.js Show resolved Hide resolved
src/wavesurfer.js Show resolved Hide resolved
@win-chen
Copy link
Contributor Author

I've been trying to figure out why the appveyor build is failing but I can't seem to replicate the bug on local builds. Do you have any advice on how to debug this?

@thijstriemstra
Copy link
Contributor

I've been trying to figure out why the appveyor build is failing but I can't seem to replicate the bug on local builds. Do you have any advice on how to debug this?

Appveyor has a mind of it's own, don't worry about it.

Copy link
Contributor

@thijstriemstra thijstriemstra left a comment

Choose a reason for hiding this comment

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

Looks good. Can you also add a changelog entry?

src/wavesurfer.js Show resolved Hide resolved
Copy link
Owner

@katspaugh katspaugh left a comment

Choose a reason for hiding this comment

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

Really nice! I like this solution a lot better than the other PR. Thank you!

@thijstriemstra
Copy link
Contributor

Really nice! I like this solution a lot better than the other PR. Thank you!

Do you want to drop the news in that pull request @katspaugh (and close it..)? Very unfortunate that contribution could not be used.

@thijstriemstra thijstriemstra merged commit 5c2c392 into katspaugh:master May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants