search

fregante / iphone-inline-video

📱 Make videos playable inline on the iPhone (prevents automatic fullscreen)
https://npm.im/iphone-inline-video
MIT License
2.05k stars 297 forks source link

Use currentSrc in emptied listener #28

Closed timdp closed 8 years ago

timdp commented 8 years ago

For video elements without an src, but with an array of <source> children, copying video.src is invalid. The getAudioFromVideo function already references video.currentSrc || video.src so I used that.

fregante commented 8 years ago

Did you have any issues specifically with this?

That listener is only called when you change the video. The specification says that you cannot actually change the video's src in any way other than through the src property itself. Your proposed check shouldn't be necessary

Dynamically modifying a source element and its attribute when the element is already inserted in a video or audio element will have no effect. To change what is playing, just use the src attribute on the media element directly, possibly making use of the canPlayType() method to pick from amongst available resources. Generally, manipulating source elements manually after the document has been parsed is an unnecessarily complicated approach.

timdp commented 8 years ago

Yep, setting the src property (or attribute) directly would definitely also work, but if you have a list of source files (let's say SD and HD representations of the same video, in both MP4 and WebM) that you want to dump into a <video> element, I think it's perfectly fine to just turn those into separate <source> elements and let the browser decide which one to play.

Of course, it won't be very efficient, given that the browser only knows the MIME type/codecs (from the type attribute of the source) and not the resolution or the bitrate. It could probe each source URL, but I doubt that mobile browsers will do this.

At any rate, it'll work as long as you call load(), and since the native API supports it, it couldn't hurt to have it in this library as well.

Thanks for merging it in, by the way. :smile:

timdp commented 8 years ago

Just to clarify: apart from the emphasized part, the quote that you added doesn't really apply to this scenario. I'm dynamically inserting source elements rather than modifying existing ones. I don't see a reason why calling load() wouldn't re-evaluate the existing ones either though.

fregante commented 8 years ago

True! I remember I had tried changing and replacing the source elements and nothing happened, but calling load() works. As per spec the currentSrc should be ready before the emptied event is fired so I'll update the code to only use that

timdp commented 8 years ago

Even better. :+1: