-
-
Notifications
You must be signed in to change notification settings - Fork 55
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
Add Support for Loading Audio Separate From Video #117
Comments
Sounds like a good enhancement. Can you maybe check what properties are/should be supported in video.js, besides |
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 -- 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?
|
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:
But that assumes the audio I hear when I play a video comes from video.js and not wavesurfer. |
It isn't, when using webaudio backend. |
No, I don't want to use |
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. |
The way to load a file or url is |
A different name for what? A src param? As I said, you should use |
I was using |
can you test replacing different audio with audio, and video with video? we can take a look at audio for video after that. |
also you shouldn't use timeout, wait until the waveform is ready and then load: player.on('waveReady', function(event) {
player.src(/* etc */);
}); |
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. |
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: |
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: 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: 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. |
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? |
I was using WebAudio. I just retested with MediaElement and the behavior is identical. |
Here is my config: Forget about all the other issues I mentioned. I don't really care about any of them, except this:
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. |
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 -- 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:
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 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. |
Thanks for those steps. Can you try the same steps with |
@DrLongGhost that last screenshot, player.src({src: 'url_of_your_file', type: 'audio/wav'}); |
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. |
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:
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
The text was updated successfully, but these errors were encountered: