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

Fix: don't use a blob URL for unknown MIME types #3471

Merged
merged 1 commit into from
Jan 8, 2024
Merged

Conversation

katspaugh
Copy link
Owner

Short description

Resolves #3380

As per this comment, if an audio file is fetched from a server that doesn't send the correct MIME type, the downloaded blob might fail to play in an audio element.

Implementation details

I've added a check when setting an audio src to a blob URL: if the MIME type is not supported, it will fallback to the original audio URL.

@katspaugh
Copy link
Owner Author

katspaugh commented Jan 7, 2024

@719media could you test this branch against the problematic server?

You can use this deployment to test: https://blob-type.wavesurfer-js.pages.dev

Copy link
Contributor

github-actions bot commented Jan 7, 2024

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@719media
Copy link

719media commented Jan 8, 2024

I have confirmed that the https://blob-type.wavesurfer-js.pages.dev/ page works for loading "bad mime-type" file examples as discussed in the linked issue. Vs current master where it continues to not work (no waveform/nothing plays)

@katspaugh
Copy link
Owner Author

Awesome, thank you! 🙏

@katspaugh katspaugh merged commit 8b1f8bb into main Jan 8, 2024
4 checks passed
@katspaugh katspaugh deleted the blob-type branch January 8, 2024 07:24
@wfk007
Copy link
Contributor

wfk007 commented Apr 17, 2024

@katspaugh Is it possible to add a config for canPlayType check? It is tough to upgrade wavesurfer version in my system because of the server side.

@katspaugh
Copy link
Owner Author

You mean to optionally skip this check?

@wfk007
Copy link
Contributor

wfk007 commented Apr 17, 2024

You mean to optionally skip this check?

Yes. we can choose whether to use this check.

@katspaugh
Copy link
Owner Author

I would suggest an alternative to that: specify a MIME type in the options. E.g.

WaveSurfer.create({
  container: '#container',
  url: 'music.mp3',
  mimeType: 'audio/mpeg',
})

It would then use this MIME type when fetching a blob:

fetch('url-to-your-data')
  .then(response => response.blob()) // Get the blob from the response
  .then(blob => {
    if (options.mimeType) {
      const newType = options.mimeType
      const dataBlob = new Blob([blob], { type: newType }); // Override the MIME type
      return dataBlob;
    }
    return blob
  })

@wfk007
Copy link
Contributor

wfk007 commented Apr 17, 2024

This seems to work for me, and this design is consistent with the record plugin(mediaRecorder.mimeType). let me have a try. Thanks for your great suggestion!

@wfk007 wfk007 mentioned this pull request Jan 13, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants