benwiley4000 / cassette

📼 A flexible media player component library for React that requires no up-front config
https://benwiley4000.github.io/cassette/styleguide
MIT License
184 stars 28 forks source link

Allow customizing media element attributes #401

Open Hypnosphi opened 5 years ago

Hypnosphi commented 5 years ago

E.g., I would like to set preload="none" instead of preload="metadata"

benwiley4000 commented 5 years ago

@Hypnosphi I'm so sorry I missed this issue when you opened it a month ago! I was in a bit of a crunch on another project, and must have missed it when it came into my email inbox. I just noticed your issue when I was reviewing the remaining issues on the repository.

Here is a short hack fix you could try while we're working on a "real solution":

// somewhere
const playlist = [...]

// in constructor
this.state = {
  preloadChanged: false
}
this.handleMediaRef = element => {
  element.preload = 'none'
  this.setState({ preloadChanged: true })
}

// in render
<MediaPlayer
  playlist={this.state.preloadChanged ? playlist : []}
  mediaElementRef={this.handleMediaRef}
  {...otherStuff}
/>

Looking through the attributes on the <video> element, I think preload is the only option we don't currently expose that makes sense to customize. I agree this could be a good feature.

We'll need to think about how to do this. Currently I believe we sort of depend on preload="metadata" since we expect to have track duration defined pretty quickly, and we'd need to make sure it's safe to lazy-load track duration.

If you'd like to open a pull request we can see how that goes! Otherwise I'll get to it in the next week or so, since this week I'm preparing to give a talk on Cassette at a conference.

I think this feature, since it has some unknowns, might belong in the 2.1.0 release which will come after the main 2.0 release (hopefully in about a month).

erikras commented 5 years ago

I think this workaround is insufficient, because the <video> element is already mounted in the dom, and fetch triggered, by the time the mediaElementRef callback is called. Or at least that's what I'm seeing.

This is very important to me because I'm counting each hit to the webpage as a podcast listen, which is incorrect. It shouldn't count as a listen until the user hits ▶️.

benwiley4000 commented 5 years ago

@erikras well if you use an empty playlist on the first render, as I did in the example workaround, there shouldn't be any fetchable source defined on the video element until you set preload="none". Let me know if there's something I missed there!

And also thanks for helping me see the motivation for the use case!

benwiley4000 commented 5 years ago

@erikras @Hypnosphi please feel free to send any update if this works or doesn't! 🙂

erikras commented 5 years ago

Here's a sandbox implementing your suggested workaround (using hooks). You can clearly see the http request for the mp3 file in your Network debugger tab. (Chrome)

Edit @cassette preload example

It should work, but maybe there's some browser quirk about using <video> with mp3s? You can even set a timeout to call the setPreloadChanged a few seconds later, and inspect the <video> element, see that it's preload="none" before the source is added, and the damn thing still fetches the file when the <source> element gets added.

benwiley4000 commented 5 years ago

I want to look at this and I'm also giving a conference talk in 6 hours so I will look this evening if I can 😄. Thanks so much for putting together a repro!

erikras commented 5 years ago

Break a leg! I'm an OSS maintainer as well, so I know there's nothing better than a sandbox to demonstrate a bug.

benwiley4000 commented 5 years ago

Wow this music is so funky lol.

I'm repro-ing the same behavior and not sure why it's happening. I can try to find some time this weekend to dig in more... it's hard to tell what's causing this.

benwiley4000 commented 5 years ago

@erikras @Hypnosphi I cloned the codesandbox locally and tried changing this:

    setPreloadChanged(true);

to:

    setTimeout(() => {
      setPreloadChanged(true);
    });

and... it worked? You no longer have the mp3 fetched until you hit play.

benwiley4000 commented 5 years ago

See here:

ezgif-4-b3de614f0012

benwiley4000 commented 5 years ago

Actually I went back and tried the same in Codesandbox, and it works there too.

benwiley4000 commented 5 years ago

You can also use requestAnimationFrame which will have better visual performance (sorry for all the comments! this is the last one I promise lol).

benwiley4000 commented 5 years ago

Hey @hypnosphi @erikras - any luck with the fix I suggested? Wanted to know if that will work for you for now.

I also definitely think we should try opening a pull request for this, so if one of you feels like authoring it, feel free - otherwise I'll get to it in the next several weeks.

erikras commented 5 years ago

@benwiley4000 Thanks for all your effort here – gives you a lot of respect for the devs at, say, YouTube, that need to make their HTML5 code work with everything – and this will certainly be useful for someone, but in the meantime, I've solved my own specific problem my own way.

Wrote about it this morning here: ⏯️ AudioCard — The React audio player designed for podcasts and Twitter cards

I don't consider us anywhere near competitors, just optimizing for slightly different objectives. I'm glad to be able to whole-heartedly recommend your project to anyone coming to me asking for more features.

benwiley4000 commented 5 years ago

This is great! Thanks for sharing.

I have a question about the Twitter thing. Does it embed the entire web UI you built inside of twitter? Is that simple to integrate? If so, it should probably be a cassette feature. I'm trying to make stuff like this a no-brainer.

erikras commented 5 years ago

No, you need to make page that is "JUST the player" and point your twitter:card meta tag value at it. e.g. https://happyhour.fm/card/003/ (and it should autoplay)

Website is OSS, too. Here's card.tsx, which invokes Player.tsx.

The important thing from the standpoint of the library author is that your UI scale perfectly to whatever Twitter wants. The library consumer has some control over the dimensions (really the aspect ratio) with the twitter:player:width and twitter:player:height values.

benwiley4000 commented 5 years ago

Ah cool! Thanks, that's pretty cool. I want to experiment with this now.. but maybe userland.

erikras commented 5 years ago

I can't be trusted to hear Github mentions, so ping me on the twitters if you need any further assistance.