Add loop prop for Video and Audio components

[DerrykBoyd]

closes: #1300

@JonnyBurger sorry for the incomplete PR, I figured this might be the best way for some async communication and hopefully get me unstuck here.

After following the general steps in the issue, here is what the current code outputs in the preview, there is a looping video, but there is also a full length video that seems to render the last frame on top of the looping videos

---

IssueHunt Summary

Referenced issues

This pull request has been submitted to:

• #1300: Loop argument on Video, Audio and OffthreadVideo

---

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
remotion	✅ Ready (Inspect)	Visit Preview	Oct 10, 2022 at 7:24PM (UTC)

[JonnyBurger]

No problem, this is the way to go!

videoDuration is a ref, which means if it gets changed, the component will not re-render. I changed it to a state now.
Next problem was with the ref's, there is a videoRef that should be passed to the native video element and a ref that should be passed to the <Video> component. When I fixed that, the time would not sync anymore so I realized a refactor needs to be done - at the very top of the Video component, we loop the whole <Video> component (not <VideoForDevelopment>) if we have the duration, which we get from a callback from the children.

It works now as intended! What's left to do is to apply the same logic to the audio code. For OffthreadVideo, I realized that there is no video tag during rendering from which we can get the duration, so my proposal is that for now, we don't support the loop property for OffthreadVideo.

Documentation is also needed :)

[DerrykBoyd]

No problem, this is the way to go!

videoDuration is a ref, which means if it gets changed, the component will not re-render. I changed it to a state now. Next problem was with the ref's, there is a videoRef that should be passed to the native video element and a ref that should be passed to the <Video> component. When I fixed that, the time would not sync anymore so I realized a refactor needs to be done - at the very top of the Video component, we loop the whole <Video> component (not <VideoForDevelopment>) if we have the duration, which we get from a callback from the children.

It works now as intended! What's left to do is to apply the same logic to the audio code. For OffthreadVideo, I realized that there is no video tag during rendering from which we can get the duration, so my proposal is that for now, we don't support the loop property for OffthreadVideo.

Documentation is also needed :)

Thanks for this @JonnyBurger, super helpful.

I added similar logic for Audio. I did run into one issue where looped audio failed to play if the duration of the audio file was longer than the sequence duration. I added the maxDuration prop as a fix, but let me know if I should do that another way.

I see the tests are failing, not quite sure why, can you give me some help there? Figured that out.

[JonnyBurger]

Thanks a lot @DerrykBoyd! It looks all good to me.

I'm ready to merge, but the thing about there being an error if the audio duration is longer than the composition did not make sense to me and I could not reproduce it, even when I removed the maxDuration logic. Could you elaborate or give me the green light if you also cannot reproduce the error anymore?

Also for statistics purposes, we are asking everybody: How long do you estimate you spent on this issue?

Cheers 🎉

[DerrykBoyd]

Thanks a lot @DerrykBoyd! It looks all good to me.

I'm ready to merge, but the thing about there being an error if the audio duration is longer than the composition did not make sense to me and I could not reproduce it, even when I removed the maxDuration logic. Could you elaborate or give me the green light if you also cannot reproduce the error anymore?

Also for statistics purposes, we are asking everybody: How long do you estimate you spent on this issue?

Cheers 🎉

Sure I can still reproduce, without the maxDuration logic, when I add the loop prop to the main audio-testing example, the audio does not play and I get the following error in the browser console

Could not play audio due to following error: DOMException: The play() request was interrupted because the media was removed from the document. https://goo.gl/LdLk22

I wasn't keeping track of the time too strictly, but I would estimate I maybe spent around 8 hours on this.

[JonnyBurger]

Thanks a lot for pointing it out, I found another two edge cases because of this:

• loop should be applied before startFrom, otherwise it muted that example
• When the duration is fetched, it should be stored globally so when it gets unmounted and mounted again, we do not have to recalculate it

The warning

Could not play audio due to following error: DOMException: The play() request was interrupted because the media was removed from the document

can now be ignored, if the duration is calculated we do a re-render, yes, but it plays the audio now.

[DerrykBoyd]

Thanks for helping me through this one @JonnyBurger, a few things were a bit of stretch as I'm still getting to grips with the internals of the package. I learned a bit though!

[JonnyBurger]

These components are super fragile, you did a good job! Thanks a lot 💙 🙌
Shipping 🚀