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

Fixing background video not playing on certain small factor devices (android) #3078

Merged
merged 2 commits into from
Nov 24, 2021

Conversation

vanch3d
Copy link
Contributor

@vanch3d vanch3d commented Nov 23, 2021

There has been reports in the past of background video not playing properly on some devices. See #1027 or #2179.

I've been facing the same issue recently when trying to play Reveal.js presentations on some IoT-type devices (android board, with WebView) and even from my phone (Samsung, Android 11, both Samsung Internet and Brave)

Could not easily reproduce the problem because of also using embedded videos (full screen) that where properly working.

Until I spotted a slight difference between the two video injection: the background video didn't have type added to the source tag generated from the data-background-video attribute in Reveal.js

The solution in this PR seems to be fixing the problem on all my devices. I cannot explain why it is working (or bugging in the first place), nor whether it will now work on every devices, including older versions of OS or browsers.

Also, worth noting that the solution in this PR is simply generating the MIME type out of the file extension, which is not a very reliable solution. But it will do, unless we want to make the specification of the data-background-video attribute more complicated

@vanch3d
Copy link
Contributor Author

vanch3d commented Nov 23, 2021

An alternative suggestion for a fix would be to have the MIME type included in the data-background-video using a secondary string delimiter, for example :

 <section
        data-background-video="http://example.com/video1.mp4:video/mp4,http://example.com/video2.webm:video/webm"
        data-background-video-loop
        data-background-video-muted
      >
        <h2>Background Video</h2>
</section>

This will still be backward compatible with the current handling of multiple sources and make the specification and validity of the MIME the responsibility of the designer, not the JS package.

@hakimel hakimel merged commit bded1f5 into hakimel:master Nov 24, 2021
hakimel added a commit that referenced this pull request Nov 24, 2021
@hakimel
Copy link
Owner

hakimel commented Nov 24, 2021

Solving this for most cases without requiring additional authoring seems like a good solution for now. We can always add the ability to provide explicit types via new syntax later.

I update the implementation a little so that we only set known/valid MIME types instead of always using the file extension. c5d5498

@hakimel
Copy link
Owner

hakimel commented Nov 24, 2021

Thanks for fixing this!

stefanocampanella pushed a commit to stefanocampanella/reveal.js that referenced this pull request Mar 1, 2022
srwohl pushed a commit to srwohl/phantom-pres that referenced this pull request Sep 2, 2024
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.

2 participants