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.2k stars 279 forks source link

Playhead fixed center #354

Closed Christilut closed 3 years ago

Christilut commented 3 years ago

This is a work in progress, just need some feedback before I can continue.

This PR adds the playheadFixedCenter option and keeps the center of the zoom waveform at the center. I've tested it with the Points API and it works well. The segments act strange though, they only seem to appear when the center is in the middle of a segment. Can't really figure out why, would like a pointer here if possible.

Another small thing I wasn't able to figure out yet: When a track is at 0 seconds, it should start at the center but now it still starts at the left side of the view. I suppose there needs to be width / 2 padding on the left of some sort but I don't know how to do that here.

And the last thing is that the current position marker in the overview needs to be adjusted by highlight width / 2 but I'm not quite sure where to get the highlight width. Is it available somewhere in the HighlightLayer or should I pass that around?

I'll add tests when everything is done.

chrisn commented 3 years ago

When a track is at 0 seconds, it should start at the center but now it still starts at the left side of the view.

What I'd probably do is allow frameOffset to go negative, which would need changes to WaveformShape._sceneFunc and/or WaveformShape._drawWaveform.

For example, if you wanted to place time 0 in the waveform 100 pixels to the right, you'd set frameOffset to -100, then in _drawWaveform, you'd avoid drawing waveform points for any negative offset.

chrisn commented 3 years ago

The segments act strange though, they only seem to appear when the center is in the middle of a segment.

To find out why segments aren't being shown correctly, I added logging to SegmentsLayer.updateSegments(). It seems that the startTime and endTime values don't correspond to the time range that's visible in the view.

chrisn commented 3 years ago

the current position marker in the overview

Because the waveform length is adjusted to fit the width of the overview view, I think the playhead should move as before rather than be at a fixed point.

(Edit) The highlight region (that shows the relative position of the zoomview waveform) seems to be doing the right thing.

chrisn commented 3 years ago

One small suggestion for this feature, perhaps the playhead position could be user-defined rather than fixed to the centre?

Christilut commented 3 years ago

Thanks! I gave it all some more thought and I think my use case is too specific for peaks so better if I start from scratch. There's too many features that I need that would probably make peaks messy :)