videojs-wavesurfer icon indicating copy to clipboard operation
videojs-wavesurfer copied to clipboard

Add Support for Loading Audio Separate From Video

Open dan24678 opened this issue 4 years ago • 25 comments

This is probably a very niche request but I need to use videojs-wavesurfer to display wavesurfer where the contents of the waveform are pulled from a different file/URL than is used for the video.

FWIW, I'll briefly explain why I need this. I want to be able to extract audio from a video file of two people speaking and process it to separate it into two channels: Person 1 on the left, and Person 2 on the right. I already have this part working.

I then want to be able to use videojs-wavesurfer to display the original video while showing wavesurfer with the results of the splitChannel audio file. It doesn't matter which version of the audio file is actually played back. I assume it would be the audio from the original non-split video, which is fine. But I need wavesurfer itself to show the splitChannel file.

I know I can do this by editing the original video to splice in the new audio, but I'm hoping to be able to avoid that approach.

I tried to use player.wavesurfer().surfer.load(url) after video.js loaded to swap in a new audio file. This does work but it seems to break the playback link between the video.js and wavesurfer so they are no longer in sync.

I looked at the code and it does seem like it might be a fairly minor change to support this, if you're willing to offer a "url" config option to support this.

It might look something like this where you could check for a "url" property on the wavesurfer config object and use that to override the "src" being used with the video:

node-app_–___src_node-app_ui_node_modules_videojs-wavesurfer_src_js_middleware_js

Note that I think this code should be above the switch case because it might apply to both backend types. In my case, I'm actually using MediaElement because of this bug in wavesurfer.

That being said, I'm happy if you can help provide a resolution to this for either backend type. So if there is some workaround that will work with the current code but not with MediaElement, that would be fine.

Any help or advice you can give would be appreciated. I'm happy to help with testing or a pull request if you can confirm there is no current workaround and that my suggested change would probably work. (Although to test it I'd need to figure out how to build this project)

Thanks, Dan

dan24678 avatar Oct 05 '20 20:10 dan24678

I looked at the code and it does seem like it might be a fairly minor change to support this, if you're willing to offer a "url" config option to support this. It might look something like this where you could check for a "url" property on the wavesurfer config object and use that to override the "src" being used with the video:

Sounds like a good enhancement. Can you maybe check what properties are/should be supported in video.js, besides src and peaks, so we can fix it properly in one go?

thijstriemstra avatar Oct 05 '20 20:10 thijstriemstra

In my case, I'm actually using MediaElement because of this bug in wavesurfer.

Could you add a one or two sentence summary of the bug in a new comment on that wavesurfer.js ticket? It'll make it a lot easier to digest the issue, the original report is quite large.

thijstriemstra avatar Oct 05 '20 20:10 thijstriemstra

@thijstriemstra -- The Wavesurfer bug is kind of unrelated but I did add a note there.

You're right about probably including peaks at the same time as src. I didn't see any other properties in that file or elsewhere that I'm aware of that should be included.

So maybe something like this above the switch statement?

            let src = this.player.wavesurfer().surfer.params.src || srcObj.src;
            let peaks = this.player.wavesurfer().surfer.params.peaks || srcObj.peaks;

            // if src was passed in through wavesurfer, call load() regardless of the backend
            if (this.player.wavesurfer().surfer.params.src) {
                this.player.wavesurfer().load(src);
                return;
            }

dan24678 avatar Oct 05 '20 20:10 dan24678

It just occurred to me that I probably don't need to be using MediaElement here since the audio playback is done through video.js and not Wavesurfer. Since my only concern with the default backend pertains to improper playback of 3+ channel audio files, this isn't an issue when playback is handled by video.js. Thus, I can use the regular backend here and the simpler change would actually be fine for me as well:

            let src = this.player.wavesurfer().surfer.params.src || srcObj.src;
            let peaks = this.player.wavesurfer().surfer.params.peaks || srcObj.peaks;

But that assumes the audio I hear when I play a video comes from video.js and not wavesurfer.

dan24678 avatar Oct 07 '20 14:10 dan24678

since the audio playback is done through video.js and not Wavesurfer.

It isn't, when using webaudio backend.

thijstriemstra avatar Oct 07 '20 14:10 thijstriemstra

this.player.wavesurfer().surfer.params.src

No, I don't want to use src param again, videojs-wavesurfer 3.x moved away from that.

thijstriemstra avatar Oct 07 '20 14:10 thijstriemstra

Oh. Then, yea, the longer fix would be better for me. But really the 3-channel thing is a minor annoyance. I'd be happy with the shorter fix, using WebAudio, and just dealing with that bug. It's not a deal breaker for me.

Can you just use a different param name? Even something namespaced for this use-case is fine. Really any way to manually inject a different URL to wavesurfer (or update it after the fact without breaking the sync) is all my use case requires.

dan24678 avatar Oct 07 '20 14:10 dan24678

Really any way to manually inject a different URL to wavesurfer (or update it after the fact without breaking the sync) is all my use case requires.

The way to load a file or url is player.src() (see https://github.com/collab-project/videojs-wavesurfer/blob/master/examples/index.html#L61). What is 'breaking the sync'? Is there a bug?

thijstriemstra avatar Oct 07 '20 14:10 thijstriemstra

Can you just use a different param name? Even something namespaced for this use-case is fine.

A different name for what? A src param? As I said, you should use player.src(). In videojs-wavesurfer 2.x there was src param and videojs's player.src() which was confusing and didn't make much sense, so it was removed in favor of the videojs way of doing things.

thijstriemstra avatar Oct 07 '20 14:10 thijstriemstra

I was using player.load() and not player.src() so let me try that and see if it works. If it does, then I should be all set. I am using 3.x.

dan24678 avatar Oct 07 '20 14:10 dan24678

No luck. I think I'm misunderstanding what you're getting at, or there is indeed a bug.

These are the two things I tried.

REPLACING WAVESURFER AUDIO VIA SURFER.LOAD() code1

Behavior: The audio and video playback buttons get out of sync. Calling surfer.play() triggers the new audio but does not trigger video playback. Clicking the video play button does nothing.

REPLACING AUDIO WITH PLAYER.SRC code2

Behavior: The video turns all black and won't play. Calling surfer.play() triggers the new audio but does not trigger video playback. Clicking the video play button triggers audio playback but the video does not play.

One thing that I don't think is related but I will mention is that the audio and video files I'm using to test his are different lengths. They won't be in the final product, this is just what I had handy. I can retest with files of identical lengths if you think that might be the cause of the bug.

Am I doing something wrong?

dan24678 avatar Oct 07 '20 15:10 dan24678

can you test replacing different audio with audio, and video with video? we can take a look at audio for video after that.

thijstriemstra avatar Oct 07 '20 15:10 thijstriemstra

also you shouldn't use timeout, wait until the waveform is ready and then load:

player.on('waveReady', function(event) {
    player.src(/* etc */);
});

thijstriemstra avatar Oct 07 '20 15:10 thijstriemstra

Sure, I'll get the correct audio file that matches the video file in length as well, just so we can not have to worry about that difference. Gimme an hour or two.

dan24678 avatar Oct 07 '20 15:10 dan24678

I'll report my tests one at a time. First, trying to swap in new video for existing video...

Result: Works as expected. The new video replaces the old one and all the playback works correctly with video.js and wavesurfer remaining synced.

There is one minor quirk. I think it probably qualifies as a wavesurfer bug, but maybe not. There are two events that I need to track: player.waveReady (the video player is ready) and wavesurfer.ready (per the docs: "When audio is loaded, decoded and the waveform drawn").

Technically, there is a race condition here. Wavesurfer doesn't exist yet before player.waveReady has fired so I cannot set its "ready" handler before starting the loading process. As it turns out, this actually is a problem and wavesurfer.ready never fires because it gets set AFTER the event occurred. It is also interesting to note that the event is not actually waiting for the waveform to be drawn. I'm using a big video file and there is a noticeable delay between when I see in my console output that wavesurfer.ready gets set and when the waveform actually appears. So it seems like the event is actually not linked to when the waveform is visible on the page. So I think maybe the docs are wrong about when the event is fired? Or something else may be going on.

At any rate (and this is interesting) when I swap in a new video with player.src(), the wavesurfer.ready event DOES fire this second time because the race condition does not apply to this second video, only to the first one.

For my purposes, I have an inline fix for the race condition and it is not a concern. Here is my code that I tested with:

node-app_–_WavesurferJs_js

dan24678 avatar Oct 07 '20 18:10 dan24678

I finished testing replacing audio with audio via player.src(). It works as expected and has no issues.

BUT it shows the video player with a black pane, which makes sense since there is no video:

VoiceVibes_Evaluate_Recording

This explains the behavior I see when I try to use player.src() to swap in new audio for a video. What it is actually doing is unloading the video and replacing everything with the audio. That's actually the behavior I would expect and want it to do. So I think that approach is probably not valid for what I need to do, which is to get the wavesurfer to show audio that is NOT taken from the video file which is being displayed.

Even when the audio and video are identical length, when I use player.wavesurfer.surfer.load() to swap in a different audio file it does not give satisfactory results. Everything works okay EXCEPT when I click a button bound to wavesurfer.play() it does not play the video. And when I play the video or move the video search bar, it does not sync with wavesurfer. Here you can see the audio and video can be put in different positions after I called player.wavesurfer.surfer.load() and broke the sync:

VoiceVibes_Evaluate_Recording

Is there a way I can manually re-sync these? That would be an acceptable workaround for me. Alternately, being able to pass in "src" to wavesurfer also addresses the issue.

dan24678 avatar Oct 07 '20 18:10 dan24678

And when I play the video or move the video search bar, it does not sync with wavesurfer.

What wavesurfer backend are you using?

Also, there is a lot to digest here, can you break it down in few bullet points, hard to summarize like this.

You also haven't shared your videojs-wavesurfer config?

thijstriemstra avatar Oct 07 '20 19:10 thijstriemstra

I was using WebAudio. I just retested with MediaElement and the behavior is identical.

dan24678 avatar Oct 07 '20 19:10 dan24678

Here is my config:

VoiceVibes_Evaluate_Recording

Forget about all the other issues I mentioned. I don't really care about any of them, except this:

  • Calling "player.wavesurfer.surfer.load()" to update wavesurfer breaks the synchronization between video-js and wavesurfer

Are you not able to replicate this? If that's the case, it might have something to do with me being inside a Vue.js component. I don't think this would matter but maybe it does.

dan24678 avatar Oct 07 '20 19:10 dan24678

can you make a codepen that reproduces the issue? also, try with a minimal setup, no ws plugins.

Have you tried https://collab-project.github.io/videojs-wavesurfer/demo/video.html btw?

thijstriemstra avatar Oct 07 '20 20:10 thijstriemstra

@thijstriemstra -- Thanks for the suggestion to use your demo page. That was much easier than having to make a code pen.

Here is a simple demonstration of the bug:

  1. Go to https://collab-project.github.io/videojs-wavesurfer/demo/video.html
  2. Paste the following into the dev console: player.wavesurfer().surfer.load('https://voicevibes.s3-us-west-2.amazonaws.com/static/video/downloaded.wav');
  3. Click on the video file slider to change the playback position. Observe that playback position of the wavesurfer waveform does not change.
  4. Paste into dev console: player.wavesurfer().surfer.play();
  5. Observe that the new audio plays but the video does not.

Technically, the new audio is much longer than the old audio it is replacing, but I know from my other testing that even with audio of identical length, the bug remains.

Do you think it is possible to fix this and make the two remain synced?


Also, I hacked away at the compiled JS file and tested the fix I was asking about originally (passing in a custom "src" argument). I had to specify to use the "WebAudio" backend to get it to load it. This has something to do with how that switch statement is constructed. Unfortunately, when I do that, it doesn't load the video. I thought this was because the switch statement didn't call next(null, srcObj); so I added that line in as well.

This gets me really close. The correct video and audio are now showing. But when I click a play button, BOTH audio files start playing.


I think it's still possible to get the behavior I want one way or another, but it doesn't seem as easy as I had initially hoped. I do want/need to get this working though, so any advice or next steps you can provide would be much appreciated.

dan24678 avatar Oct 08 '20 15:10 dan24678

Here is a simple demonstration of the bug:

Thanks for those steps. Can you try the same steps with player.src() and report results? Using the wavesurfer instance directly, e.g. player.wavesurfer().surfer is not encouraged, but for testing/debugging, sure.

thijstriemstra avatar Oct 08 '20 15:10 thijstriemstra

Wavesurfer_Plugin_Video_Example

It throws an error because the player is in video mode and we are trying to load an audio file.

dan24678 avatar Oct 08 '20 15:10 dan24678

@DrLongGhost that last screenshot, player.src() expects an object, try this instead:

player.src({src: 'url_of_your_file', type: 'audio/wav'});

thijstriemstra avatar Nov 01 '20 13:11 thijstriemstra

Using player.src() to swap in a different audio file results in the Video playback window resetting its duration counter to 00:00/00:00 and no longer playing the video when my wavesurfer play button is clicked, so this isn't a working solution for me.

dan24678 avatar Nov 11 '20 15:11 dan24678