bbc / peaks.js

JavaScript UI component for interacting with audio waveforms
https://waveform.prototyping.bbc.co.uk
GNU Lesser General Public License v3.0
3.21k stars 281 forks source link

2 Feature Requests #329

Closed sonicviz closed 4 years ago

sonicviz commented 4 years ago

Hi,

Just exploring the library and demo's and have a couple of thoughts for useful additions.

  1. Looping on segments. UI controls could be a "global" loop checkbox and/or individual loop segment checkboxes on the log segments/points display on the demo.
  2. Time stretching via https://github.com/cutterbl/SoundTouchJS. See also example at https://github.com/ZVK/stretcher

Thanks for the cool tools!

sonicviz commented 4 years ago

Scratch #2, figured that out.

Looping would be cool though.

sonicviz commented 4 years ago

Would it be possible to change the speed of the playback cursor?

chrisn commented 4 years ago

Looping would be cool though.

Would it be good enough to add a loop flag to player.playSegment() which, if set, does a seek to the segment start when playback reaches the segment end? Or does your use case require sample accurate looping?

chrisn commented 4 years ago

Would it be possible to change the speed of the playback cursor?

I haven't tested this, but if the playhead marker is out of sync if you adjust the media element's playbackRate property, I'd consider that a bug we should fix.

Beyond that, I haven't looked at the time stretching libraries you mention. I am open to considering adding extension points to Peaks.js to support such use cases, but I don't see time stretching as a core feature for this library.

sonicviz commented 4 years ago

Would it be good enough to add a loop flag to player.playSegment() which, if set, does a seek to the segment start when playback reaches the segment end? Or does your use case require sample accurate looping?

I don't think sample accurate is needed, as long as it's close enough. The use case here is looping short musical riffs to learn them, so as long as it's with a few ms should be fine.

I haven't tested this, but if the playhead marker is out of sync if you adjust the media element's playbackRate property, I'd consider that a bug we should fix.

Ah, right, let me try the playbackRate , I missed that.

Beyond that, I haven't looked at the time stretching libraries you mention. I am open to considering adding extension points to Peaks.js to support such use cases, but I don't see time stretching as a core feature for this library.

No problem. Best to have focus, and Peaks focus is excellent.

chrisn commented 4 years ago

If you haven't seen it, please have a look at our docs on customizing Peaks.js. There's a section that talks about using Web Audio API based media players, which could possibly be used?

sonicviz commented 4 years ago

Yeah, I saw that. I took a long look at the tone js demo, but I can get what I need done with the HTML MediaElement ok. Thanks!

From: Chris Needham notifications@github.com Subject: Re: [bbc/peaks.js] 2 Feature Requests (#329)

If you haven't seen it, please have a look at our docs on customizing Peaks.js. There's a section https://github.com/bbc/peaks.js/blob/master/customizing.md#media-playback that talks about using Web Audio API based media players, which could possibly be used?

sonicviz commented 4 years ago

audioPlayer.playbackRate works prefectly to keep it in synch. Thanks!.

chrisn commented 4 years ago

I just pushed a change to add looped segment playback. Please let me know if this works OK for you!

sonicviz commented 4 years ago

Thanks, I'll take a look at it asap.

sonicviz commented 4 years ago

Sorry for the late reply, just getting round to updating this now and testing it. This loop fix hasn't been pushed to NPM? https://www.npmjs.com/package/peaks.js?activeTab=readme It's not showing up when I run npx npm-check -u

chrisn commented 4 years ago

I haven't published this change to npm yet. I wanted to hear your feedback before doing that. To test it, you can install directly from GitHub master branch.

sonicviz commented 4 years ago

Ok, tested it and works fine.

One issue though. Because I have implemented time stretching, I need an event trigger when the loop restarts, so I can reset the percentage played - otherwise it just keeps playing when time stretching is enabled.

Is there an event to go with this? Had a quick look but didn't see one. Something similar to player.play but player.looped if that makes sense?

chrisn commented 4 years ago

There is a player.seeked event, which is emitted each time it loops.

sonicviz commented 4 years ago

That works, thanks!

Ran into another issue though, not sure if it's peak.js or me.

I subscribed to player.play, player.seeked, and player.paused and logged them to console to check.

If I create a segment and use the audio control to stop/start then player.play gets triggered on play & player.paused gets triggered on stop correctly.

If I log the segment, and use log play to start playback -> player.play, player.seeked & player.paused get triggered (seeked gets triggered with the play)

If I use log.loop to start playback-> player.play, player.seeked & player.paused get triggered as in log play, but...and here's the issue...if I use the audio control to stop loop playback it stops but player.seeked is continually still getting triggered.

If I start to loop again with log loop to start playback and then use log play to stop it then it stops correctly at the end of the segment and the player.seeked events are then stopped correctly also.

Does that make sense?

So I need to be able to kill the player.seeked events if I use audio control to stop loop playback.

chrisn commented 4 years ago

Yes, I'm seeing that too. I'll investigate...

sonicviz commented 4 years ago

Great, thanks for the confirmation!

chrisn commented 4 years ago

The looping is now fixed.

sonicviz commented 4 years ago

Just tested and confirm fix. Awesome, tyvm! Nice work.

chrisn commented 4 years ago

Great, I'll prepare a new release for npm. Is it OK to close this issue?

sonicviz commented 4 years ago

Great, Ty.