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

Zoom optimisation for Waves and matching implementation for Spectrograms #2646

Merged
merged 31 commits into from
Mar 11, 2023

Conversation

luke-harrison-personal
Copy link

@luke-harrison-personal luke-harrison-personal commented Jan 17, 2023

Zoom optimisation for Waves and matching implementation for Spectrograms

Zoom_Example

Short description of changes:

My goal was to allow zooming on a spectrogram to be performed faster and with no maximum width. To do this I implemented optimisations in both the wavesurfer and the spectrogram with the intention of making zooming better for users.

Removing the maximum width

I took the multicanvas approach used in the wavesurfer drawer and applied it to the spectrogram so that the maximum size limit would be circumvented. In my approach all canvases are equally sized as I found this to be easier to manage, however the multicanvas drawer remains the same.

Improving Performance

Existing canvases can have their width changed in style without redrawing/recalcuation. This is an incredibly fast way to preview a wave/spectrogram at various zoom levels. I created a zooming() function to be called when the zoom level is changed, and only calling the original zoom() function when the zoom slider is released. This approach preserved the original behavior of zoom for older implementations.

The spectrogram is still a very computationally expensive task and so generating particularly large ones takes a lot of time. I scheduled canvases on a priority so that viewable and near-viewable canvases would be rendered before others. This process can be seen when zooming out quickly after zooming in. This was applied to the wavesurfer too, although it was much less common for long waves to generate the same level of lag as the spectrogram

Spectrogram_Priority_Example

An example of zooming while the audio is playing:
Zoom_Play_Example

Breaking in the external API:

None that I know of

Breaking changes in the internal API:

None that I know of

Todos/Notes:

Todos

None

Notes

Some unrelated changes were necessary as a result of the approach taken. Since the drawer width changes with the zoom, I had to remove the check in drawer.js that prevented redrawing the wave if the zoom level remained unchanged.

I changed the example pages for zoom and spectrogram to reflect the new implementation.

This implementation adds/removes no files and requires no additional libraries.

Related Issues and other PRs:

None that I know of

Luke Harrison and others added 30 commits November 29, 2022 09:37
redrawing/recalculating to reduce CPU requirements while zooming in on
waveforms.
Currently displaying a waveform image in example/zoom which will be used
to hide the lack of wave drawing while zooming.
TO DO:
Hide the real wave while zooming
Hide the backimage after zooming
Zoom in on current timestamp while zooming on backimage
FUTURE:
Split or crop backimage for longer waves
Re-render backimage to match the colour style of the wave with selected
timestamp
Since the coloured section of the wave is also handled by canvas objects
within a wave I am going to move the backimage functionality into the
canvasentry.js file so that each canvas handles it's own image, which
should resolve the issue as well as segment the image into sections.
the left offsets of each backimage are not being calculated correctly
and images are overlapping.
Creates backimages for progress wave
Left offset of canvas elements are handled correctly.
Zooming in needs a viewable region boundary implemented so that canvases
don't become too large.
Progress tracking and regions plugin work without any issues now.
Canvases not in frame are cleared and not rendered
Canvases that become too large are clipped
Progress location is maintained
Currently, changing zoom level before all canvases have been loaded
causes issues, my next goal is to fix this by either improving the
handling of missing canvases or preventing zoom change until all
canvases have loaded.
I'll use and hopefully instead of locking up I can return to a backup
render of the wave.
This is done by cancelling loading and showing only loaded sections
Zooming out on partially loaded waves loads a backup image, this
features works regardless of wave size.
With this change the branch now passes the included tests
Low zoom level previously caused issue with back image generation, the
issue has been resolved.
This will allow testing of the intended multicanvas implementation
Canvases are equally sized
Calculation is on the whole spectrogram and then converted to seperate
canvases
This is a starting point for scaling up to an adaptive number of
canvases, similar to the multicanvas drawer used for the main wave.
Ideally calculation can be moved such that it is done for each canvas,
this would mean speed could be improved by calcuating visible
spectrogram canvases.
Right now this offers no noticeable speed improvement, but could improve
the ability to display zoomable spectrograms without creating images
too large for browsers.
Image still generated first then split, limiting max size
Committing a working version before attempting fix
Currently trying to draw all canvases at once causes failure
Next I'm going to implement staggering of the canvases to hopefully
allowing further zooming even without performance improvements
Fixed an issue where resample() was creating full-size arrays instead of
limiting to the canvas size
Fixed an issue where render() was being called twice as a result of an
incorrect event fire in wavesurfer.js
Removed placeholder text in zoom example
After this I will implement delays rendering of low-priority canvases
Fixed issue where progress wasn't accurate during zoom
Removed functions that were no longer necessary
backimages are no longer required, existing canvases are stretched
during zoom.
backup image is no longer required, canvases can continue rendering
during the zooming process.
Removed/changed comments referring to the removed backimage/backupimage
Upon first call of the stretchCanvases() function the drawer will switch
over to the optimised zoom functionality
@luke-harrison-personal luke-harrison-personal marked this pull request as ready for review January 18, 2023 00:20
@coveralls
Copy link

coveralls commented Jan 26, 2023

Coverage Status

Coverage: 79.253% (-1.9%) from 81.108% when pulling 0c550f0 on luke-harrison-personal:zoom_optimisation into 5a772c8 on katspaugh:master.

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.

I think it's a pretty clever solution, basically stretching an already drawn waveform while a new one is being calculated. Personally not a fan how the stretched one looks but it's definitely smoother!
The delayed rendering and prioritization of canvases is also a really good idea, regardless of zooming, actually.
And finally, adding multi-canvas to the spectrogram is no question an excellent addition.
Overall, while I think this comes with a lot of extra complexity, it's a solid UX improvement. 👍

@entonbiba
Copy link
Contributor

@katspaugh @thijstriemstra @luke-harrison-personal been looking into this as well lately. We can look into adding css image-rendering: pixelated; so it doesn't blur the canvas while it's stretching it. There is also the globalConpositeOperation in canvas we could potentially utilize.
https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/globalCompositeOperation

However there is also the possibility of making these peaks as SVG in realtime as an idea.

@katspaugh
Copy link
Owner

That CSS rule is worth trying 👍
SVG, unfortunately, didn't fare well in terms of performance back when we used to have it as a rendering option. Things might’ve changed. In any case, it’s outside the scope of this PR.

@katspaugh katspaugh merged commit 7c1458a into katspaugh:master Mar 11, 2023
This was referenced Mar 12, 2023
katspaugh added a commit that referenced this pull request Mar 24, 2023
@katspaugh katspaugh mentioned this pull request Mar 24, 2023
katspaugh added a commit that referenced this pull request Mar 26, 2023
katspaugh added a commit that referenced this pull request Mar 27, 2023
aliking added a commit to animoto/wavesurfer.js that referenced this pull request May 11, 2023
* Fix clientWidth error in responsive mode (katspaugh#2499)

* Fixes potential error with clientWidth on resizing

* add changelog entry

* cursor plugin: fix typeError if displayTime: null (katspaugh#2512)

* Cursor Plugin: fix typeError if displayTime: none

* add changelog entry

* spectrogram plugin: fix for hi-dpi displays and independent height (katspaugh#2509)

* Improved behavior on retina displays

* Update Spectrogram example with `height`

* Update changelog

* release: 6.2.0 (katspaugh#2514)

* update changelog

* bump version: 6.2.0

* update dev dependencies

* update dev dependencies

* doc: remove nft from the readme (katspaugh#2555)

* update dev dependencies

* regions plugin: restore support for one drag selection for all channels (katspaugh#2551)

* regions plugin: restore support for one drag selection for all channels

* Minor: split a comment into 2 lines

* Add entry in the changelog

* Draw all bars shorter than barMinHeight with the specified minimum height (katspaugh#2523)

* fix katspaugh#2522

* katspaugh#2523 add comment and change log entry

* markers plugin: add support for a context menu event on a marker (katspaugh#2546)

* Add support for a context menu event on a marker

* Fix formatting

* Remove event listeners on marker delete

* Add changelog entry

* Formatting changes

* Fix changelog entry formatting

* spectrogram plugin: fix issue where labels get stuck when scrolling (katspaugh#2558)

* Fix floating labels (issue katspaugh#2542)

Change display from 'fixed' to 'absolute' to correct position when scrolling

* Update CHANGES.md for PR katspaugh#2558

* doc: fix typos in exportImage api doc

* build: remove unused babel-plugin-proxy dependency (katspaugh#2568)

* update dev dependencies

* bump version: 6.3.0

* feat(markers): add tooltip (katspaugh#2595)

* feat: tooltip

* fix: semi

* Add a GitPOAP badge in the readme (katspaugh#2594)

* markers plugin: check for event after every add/remove (katspaugh#2599)

* katspaugh#2560 check for event after every add/remove

* Updated for lint issues

* Fixed events after re-registration

* Add changelog entry

* update changelog

* ci: update actions and test against node.js 16.x

* update dev dependencies

* cursor plugin: fix crash when destroy triggered before ready (katspaugh#2606)

* cursor plugin: fix crash when destroy triggered before ready (katspaugh#2602)

* update changelog,update inline comments

* fix indentation

* add parentheses

Co-authored-by: Thijs Triemstra <info@collab.nl>

* release: 6.4.0 (katspaugh#2614)

* update dev dependencies

* bump version: 6.4.0

* doc: fix typo

* update dev dependencies

* regions plugin: improved delta calculation (resize end) (katspaugh#2641)

* Improved delta calculation (resize end)

With the use case that I use this library for, users need to be able to resize the regions. When a region was resized and afterward set back to the min length (f.e. 3.1 sec or in HMSM, which is how we show it, 00:00:03:10) it would stay at 3.12sec (00:00:03:12) or 3.14sec (00:00:03:14) or something, but never go back down all the way to 3.1 (00:00:03:10). With these changes, the length of the region can be set back to 3.10 exactly

* Update: code feedback implemented

* Add: changelog entry

* Create FUNDING.yml (katspaugh#2668)

* Docs: update the issue template and wavesurfer links (katspaugh#2671)

* Docs: update the issue template

* Update the readme

* katspaugh/wavesurfer.js -> wavesurfer-js/wavesurfer.js

* Proxy HTMLMediaElement's 'waiting' event through MediaElement backend. (katspaugh#2691)

* Proxy HTMLMediaElement's 'waiting' event

* fix

* Fix iphone silent switch webaudio mute (katspaugh#2686)

fix iphone silent switch mode when using webaudio so it continues playing audio

* Update CHANGES.md

* Fix typo (katspaugh#2695)

* Docs: add an import snippet (katspaugh#2700)

* Update FUNDING.yml (katspaugh#2704)

Change the sponsorship account to wavesurfer-js

* inherit font family rather than setting monospace (katspaugh#2664)

Co-authored-by: Sishaar Rao <sishaar@nooks.in>

* regions plugin: add editableContent and removeButton options (katspaugh#2521)

* regions new features: editableContent, removeButton
with example how it works

* fix katspaugh#2521 request changes

* add removeEventListener to each addEventListener

---------

Co-authored-by: Thijs Triemstra <info@collab.nl>

* [FIX] respect mute state when changing volume during mute (katspaugh#2503)

* [FIX] respect mute state when changing volume during mute

@see katspaugh#2502

* change unreleased version from to 6.2.0

---------

Co-authored-by: Thijs Triemstra <info@collab.nl>
Co-authored-by: katspaugh <381895+katspaugh@users.noreply.github.com>

* release: 6.5.0 (katspaugh#2705)

* release: 6.5.0

* Rm duplicates in the changelog

* Add a better description for funding.yml

* Spelling

* Fix: release workflow permissions (katspaugh#2709)

* Fix: github repo URL

* Restore branch name

* Zoom optimisation for Waves and matching implementation for Spectrograms (katspaugh#2646)

* Used setTimeout() method to require a minimum time period passes before
redrawing/recalculating to reduce CPU requirements while zooming in on
waveforms.
Currently displaying a waveform image in example/zoom which will be used
to hide the lack of wave drawing while zooming.

* Implemented a backimage mimicking the wave to display zoom level faster
TO DO:
Hide the real wave while zooming
Hide the backimage after zooming
Zoom in on current timestamp while zooming on backimage
FUTURE:
Split or crop backimage for longer waves
Re-render backimage to match the colour style of the wave with selected
timestamp

* Added hiding/showing of elements while zooming
Since the coloured section of the wave is also handled by canvas objects
within a wave I am going to move the backimage functionality into the
canvasentry.js file so that each canvas handles it's own image, which
should resolve the issue as well as segment the image into sections.

* backimages are stored and drawn for each canvas entry. Current issue is
the left offsets of each backimage are not being calculated correctly
and images are overlapping.

* Zooming now correctly handles progress positions.
Creates backimages for progress wave
Left offset of canvas elements are handled correctly.

* Zooming out now works without any issues
Zooming in needs a viewable region boundary implemented so that canvases
don't become too large.
Progress tracking and regions plugin work without any issues now.

* Zoom optimisations have been made
Canvases not in frame are cleared and not rendered
Canvases that become too large are clipped
Progress location is maintained

* Added priority drawing for canvases in view
Currently, changing zoom level before all canvases have been loaded
causes issues, my next goal is to fix this by either improving the
handling of missing canvases or preventing zoom change until all
canvases have loaded.

* Zoom slider locks up if rendering isn't finished
I'll use and hopefully instead of locking up I can return to a backup
render of the wave.

* Allows zooming immediately after release
This is done by cancelling loading and showing only loaded sections

* Improved backup image that can handle large waves
Zooming out on partially loaded waves loads a backup image, this
features works regardless of wave size.

* Optimisation checks if there is a wrapper
With this change the branch now passes the included tests

* Fixed issues with back image generation
Low zoom level previously caused issue with back image generation, the
issue has been resolved.

* Added zooming to spectrogram example
This will allow testing of the intended multicanvas implementation

* Spectrogram successfully split into two canvases
Canvases are equally sized
Calculation is on the whole spectrogram and then converted to seperate
canvases
This is a starting point for scaling up to an adaptive number of
canvases, similar to the multicanvas drawer used for the main wave.
Ideally calculation can be moved such that it is done for each canvas,
this would mean speed could be improved by calcuating visible
spectrogram canvases.
Right now this offers no noticeable speed improvement, but could improve
the ability to display zoomable spectrograms without creating images
too large for browsers.

* Working for arbitrary number of canvases
Image still generated first then split, limiting max size
Committing a working version before attempting fix

* spectrogram sections are generated per-canvas
Currently trying to draw all canvases at once causes failure
Next I'm going to implement staggering of the canvases to hopefully
allowing further zooming even without performance improvements

* Spectrogram can now display at unlimited width
Fixed an issue where resample() was creating full-size arrays instead of
limiting to the canvas size
Fixed an issue where render() was being called twice as a result of an
incorrect event fire in wavesurfer.js
Removed placeholder text in zoom example

* Spectrogram only draws viewable canvases
After this I will implement delays rendering of low-priority canvases

* Rearranged variables to allow for setting delays

* Canvases stretch to zoom

* Zoom now scales correctly against waveform
Fixed issue where progress wasn't accurate during zoom

* Improved zooming functionality for wavesurfer
Removed functions that were no longer necessary
backimages are no longer required, existing canvases are stretched
during zoom.
backup image is no longer required, canvases can continue rendering
during the zooming process.

* Implemented priority rendering for spectrogram
Removed/changed comments referring to the removed backimage/backupimage

* Added timeout clearing to prevent drawing old canvases after zoom level
changes

* Removed unecessary class variables

* Patched in a fix so that original zooming behaviour is preserved
Upon first call of the stretchCanvases() function the drawer will switch
over to the optimised zoom functionality

* Removed unecessary changes to formatting

* Removed more unecessary changes

---------

Co-authored-by: Luke Harrison <s4587520@student.uq.edu.au>

* [Regions] Fix undefined content element on remove (katspaugh#2713)

Co-authored-by: GitHub Actions <github@action>

* Chore: improve the release and changelog automation (katspaugh#2710)

* Chore: automate changelog

* Update the readme wrt releasing

* Fix changelog

* Readme

* Rm production, use master

* Fix commit message

* Fix sed

* List syntax: * -> -

* Exit on error

---------

Co-authored-by: GitHub Actions <github@action>

* Release 6.6.0 (katspaugh#2715)

* Release 6.6.0

* Update CHANGES.md

* Docs: add a video tutorial link to the readme (katspaugh#2724)

* Fix: NPM publish in the CI job (katspaugh#2727)

* Fix: NPM publish in the CI job

* Test on this branch

* Dependencies

* Revert "Test on this branch"

This reverts commit 5d9f92f.

* if success()

* Fix: avoid exit 1 in CI script (katspaugh#2734)

* Fix: avoid exit 1 in CI script

* Temp: test on this branch

* Release 6.6.1

* Test release

* Rm temp test

* Refactor: remove scriptNode (katspaugh#2706)

* feat(MarkersPlugin):  add getMarkers function (katspaugh#2743)

* Revert "Zoom optimisation for Waves and matching implementation for Spectrograms (katspaugh#2646)" (katspaugh#2744)

This reverts commit 7c1458a.

* Release 6.6.2 (katspaugh#2745)

* Docs: version the unpkg script in readme (katspaugh#2746)

* Remove marker by id (katspaugh#2749)

* remove marker by id

* better implementation

* remove extra whitespace

* Release 6.6.3 (katspaugh#2753)

* Docs: add a note about wavesurfer.ts (katspaugh#2760)

* fix: relay dblclick event (katspaugh#2761)

* Bump yaml from 2.2.1 to 2.2.2 (katspaugh#2766)

Bumps [yaml](https://github.com/eemeli/yaml) from 2.2.1 to 2.2.2.
- [Release notes](https://github.com/eemeli/yaml/releases)
- [Commits](eemeli/yaml@v2.2.1...v2.2.2)

---
updated-dependencies:
- dependency-name: yaml
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump engine.io from 6.4.1 to 6.4.2 (katspaugh#2771)

Bumps [engine.io](https://github.com/socketio/engine.io) from 6.4.1 to 6.4.2.
- [Release notes](https://github.com/socketio/engine.io/releases)
- [Changelog](https://github.com/socketio/engine.io/blob/main/CHANGELOG.md)
- [Commits](socketio/engine.io@6.4.1...6.4.2)

---
updated-dependencies:
- dependency-name: engine.io
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* update node/yarn

* update docker node

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Milton Läufer <miltonlaufer@gmail.com>
Co-authored-by: Mikołaj Zatorski <41756225+MikeyZat@users.noreply.github.com>
Co-authored-by: mjrond <82426028+mjrond@users.noreply.github.com>
Co-authored-by: Thijs Triemstra <info@collab.nl>
Co-authored-by: katspaugh <381895+katspaugh@users.noreply.github.com>
Co-authored-by: Muhammad Sayed <muhammad.s.mahdy@gmail.com>
Co-authored-by: johannes <58665953+drinking-code@users.noreply.github.com>
Co-authored-by: kpanditCtr <108754847+kpanditCtr@users.noreply.github.com>
Co-authored-by: David M. Weigl <musicog@users.noreply.github.com>
Co-authored-by: Matteo Pietro Dazzi <matteopietro.dazzi@gmail.com>
Co-authored-by: Roman Štefek <info@beaverlyhills.eu>
Co-authored-by: iShirKhan <46706370+SilvaQ@users.noreply.github.com>
Co-authored-by: BQTH <47450112+BQTH@users.noreply.github.com>
Co-authored-by: Anri Asaturov <anri82@gmail.com>
Co-authored-by: Enton Biba <entonbiba@users.noreply.github.com>
Co-authored-by: Stefan de Konink <stefan@konink.de>
Co-authored-by: Sishaar Rao <sishaar@gmail.com>
Co-authored-by: Sishaar Rao <sishaar@nooks.in>
Co-authored-by: Rustam Apay <rustamapaev@gmail.com>
Co-authored-by: othmar52 <othmar52@users.noreply.github.com>
Co-authored-by: luke-harrison-personal <84890777+luke-harrison-personal@users.noreply.github.com>
Co-authored-by: Luke Harrison <s4587520@student.uq.edu.au>
Co-authored-by: GitHub Actions <github@action>
Co-authored-by: Fabian Obermaier <fabian.obermaier@yahoo.de>
Co-authored-by: prakharpbuf <54688220+prakharpbuf@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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

5 participants