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

feat: create multitrack with media element #21

Merged
merged 2 commits into from
Aug 30, 2023
Merged

Conversation

WooWan
Copy link
Contributor

@WooWan WooWan commented Aug 29, 2023

Short description

This pull request allows the creation of a Multitrack using the media parameter.

Resolves https://github.com/katspaugh/wavesurfer-multitrack/issues/19

Implementation details

During audio initialization, the code first checks url parameter first and if it is not present, it will then look for the media parameter. This ensures backward compatibility, as the behavior remains the same even if both url and media parameters are provided by the user.

Additionally, the code has been updated to switch from using track.url to track.url || track.options?.media.

How to test it

You can test it as shown below

const multitrack = Multitrack.create(
  [
    {
      id: 0,
      url: '/music/mp3/drum.mp3', //your audio file
      volume: 0.5,
      startPosition: 0,
      options: {
        media: new Audio('/music/mp3/vocal.mp3'),
        height: TRACK_HEIGHT,
        waveColor: 'hsl(46, 87%, 49%)',
        progressColor: 'hsl(46, 87%, 20%)'
      }
    }
  ],
  {
    container: playerRef.current, // required!
    minPxPerSec: 5, // zoom level
    rightButtonDrag: true, // drag tracks with the right mouse button
    cursorWidth: 2,
    cursorColor: '#D72F21', 
    trackBorderColor: 'black',
    envelopeOptions: {
      lineWidth: '0',
      dragPointSize: 0
    }
  }
);

Screenshots

Now it works!
image

Checklist

  • This PR is covered by e2e tests
  • It introduces no breaking API changes

@@ -131,9 +131,15 @@ class MultiTrack extends EventEmitter<MultitrackEvents> {
}

private initAudio(track: TrackOptions): Promise<HTMLAudioElement> {
const audio = new Audio()
let audio = new Audio()
Copy link
Owner

@katspaugh katspaugh Aug 29, 2023

Choose a reason for hiding this comment

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

I would move the audio creation to the if. I.e.

    let audio: Audio
    if (track.url) {
       audio = new Audio()
       audio.crossOrigin = 'anonymous'
       audio.src = track.url
     } else if (track.options?.media) {
       audio = track.options?.media
     }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I don't initialize Audio() early on, an error message appears, saying Variable 'audio' is used before being assigned. I believe that audio should still be created even if the user doesn't provide either the url parameter or the options.media parameter.

Alternatively, other approaches come to mind, such as adding an else statement or using type assertion. However, early initialization of audio seems like the better option to me. If I'm misunderstanding something or if there's a better approach, please let me know.

error message
image

Copy link
Owner

Choose a reason for hiding this comment

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

Correct, it should create an audio element without src if neither url nor media are provided. So yeah, an else clause at the end would work.

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.

Looks good, thank you! Just one small suggestion.

@WooWan WooWan requested a review from katspaugh August 30, 2023 05:35
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.

Thanks! 👍

@katspaugh katspaugh merged commit f0b2b5d into katspaugh:main Aug 30, 2023
@PriyavKaneria
Copy link

Thanks for this! There are no Issues on this repo so came looking here for solution
I was using the MultiTrack along with a StreamResponse of audio, and constantly faced the below error
Failed to set the 'currentTime' property on 'HTMLMediaElement'

image

The issue I was able to debug was the StreamResponse was using header Transfer-Encoding: chunked reference
which inherently does not give complete content length leading to the duration field being set as "Infinity" leading to errors on the seekTo and updatePosition functions due to bad t4 value in initAllAudios

The solution was to first fetch the audio as Blob and create a URL which can be then used using this media option on the track

const loadSong = async () => {
    const src = songURL;
    const blob = await fetch(src).then((resp) => resp.blob());
    const blobURL = URL.createObjectURL(blob);
    multitrack.addTrack({
	  id: 0,
	  startPosition: 0,
	  draggable: false,
	  options: {
		  media: new Audio(blobURL),
		  waveColor: '#ed738e',
		  progressColor: '#e11d48',
		  dragToSeek: true,
		  plugins: []
	  }
    });
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants