add --height --width flags

[exitflynn]

this references issue #808

I tried implementing as advised and came up with the following. Am I moving in the right direction? I'll be extending this to the lambda and still commands and documenting afterwards.

---

IssueHunt Summary

Referenced issues

This pull request has been submitted to:

• #808: Dynamically set width and height & FPS

---

[vercel]

@exitflynn is attempting to deploy a commit to the Remotion Team on Vercel.

A member of the Team first needs to authorize it.

[exitflynn]

woah sir i must inform you that this is a set-up and a ruse, i have no intention of attempting to deploy a commit on vercel.

haha, that happened by mistake, any way i can prevent that from happening the next time?

[vercel]

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

Name	Status	Preview	Updated
remotion	✅ Ready (Inspect)	Visit Preview	Oct 28, 2022 at 9:42AM (UTC)

[JonnyBurger]

Haha all good, this happens automatically on every pull request, and gives a preview link to the website, so we can read for example the new documentation!

I will take a closer look at the documentation tomorrow!

[exitflynn]

hey @JonnyBurger, I have only been able to implement the flags in the cli commands so far, and thought creating a PR would make for a convenient way to show/discuss those changes (i now realise a Draft PR is what I was looking for). I'm currently trying to figure out how to port these functions to lambda and still, I'll be documenting what I've done so far today. (i also have to make sure the h264 validation remains un-bypassable) 💯 👍

thank you!

[JonnyBurger]

@exitflynn Awesome, thanks a lot! A quick hint on how to continue:

You can now pull out width and height from the CLI options: https://github.com/exitflynn/remotion/blob/6024cc6ddfd3c4aabf28463a3936c6b0cea3b434/packages/cli/src/render.tsx#L89 and here you can overwrite the default width and height whic in the config object: `https://github.com/exitflynn/remotion/blob/6024cc6ddfd3c4aabf28463a3936c6b0cea3b434/packages/cli/src/render.tsx#L186

This should already result in a working proof of concept :)

[exitflynn]

I had been absolutely s t u c k for > the past two days. I tried looking at other cli flags like concurrency or audio-bitrate to see how they were implemented to gain more insights into this one but kept running into all sorts of headblocks.

Thanks a lot for reaching out with that hint! It looks pretty simple in hindsight now, had me reverting some commits. I also finally managed to overwrite the values and make the cli-flags work for render, with the h264 validation still holding 🥳 Still have to figure out how the same thing should be implemented in lambda and still. Will get to it tomorrow, it's so late that it's early here :P

Thanks again, @JonnyBurger!

[exitflynn]

Hey @JonnyBurger, I managed to figure out how to overwrite the height and width for the still image rendering and the lambda rendering to whatever we may want (with the validation holding) but I have been trying all day and having trouble getting those values from the CLI for the two cases (among other things, i tried seeing if there was a way the parsedCLI values could be used over here but I am unsure if parseCommandLine()/parse-command-line.ts gets called in the first place for still and lambda 🤔).

Could you please provide some pointers / locations in the codebase as to where I should look (for getting input from CLI flags for still and lambda commands)? It'd be of immense help. 😅

[JonnyBurger]

Hey @exitflynn!

I don't know the reason for the desparation, you actually implemented it correctly! :)
I was able to change the the dimensions of a render by passing the --height and --width parameters to the command line. This was the main point of the PR!

I have helped a bit with some stronger validation and passing all the parameters through to Lambda to also make the feature work there.

What's left is to add the new parameters to the documentation (for renderMedia(), renderStill(), renderStillOnLambda(), renderMediaOnLambda(), npx remotion render, npx remotion lambda render, npx remotion still, npx remotion lambda still) and then that's it! If this is done, I'll be happy to approve the PR 🙂

[exitflynn]

hahahaha thank you soo much @JonnyBurger! After trying all day, I finally (or atleast i think i) got the hang of it and I was able to implement the functionality to lambda (in a rough manner, was going to flesh things out later, including the render ones) on my own! (well, partially).Though I was still a bit stumped by the still (pun not intended) one, I was going to reply what started with a "nevermind that" but I saw your response and was very pleasantly surprised. (pushed what I had been upto to a different branch cos why not :P)

I'm looking over the additions and the changes you made and they make so much more sense. For instance, it is clearly obvious how something as small as renaming the CLI height to forcedHeight would considerably improve readability.
Thank you so much again for being so tremendously helpful! I'll be getting on the documentation right away!

[exitflynn]

had to leave for classes so pushed but this is NOT the final documentation haha

[JonnyBurger]

Nice, you were on the right track as well, sorry for interfering!
Waiting for the final changes, but the PR is on a good track 😇

[exitflynn]

Haha thanks a lot for everything, @JonnyBurger! Quick question, I added the documentation but I'm unsure which release number to put like here (i just put v3.2.40, is that something that gets added on later?).

Also would it look nicer if I described the --height and --width together instead of two separate entries?

And I was also wondering whether I need to make an entry for height and width in render-frames.md or not 🤔. Thank you as always, for being so supportive! 🤠

[JonnyBurger]

Haha thanks a lot for everything, @JonnyBurger! Quick question, I added the documentation but I'm unsure which release number to put like here (i just put v3.2.40, is that something that gets added on later?).

3.2.40 sounds good! I'll adjust it manually if it gets added into a later release.

Also would it look nicer if I described the --height and --width together instead of two separate entries?

Good question, the convention for now is to keep it separate, might change it in the future

And I was also wondering whether I need to make an entry for height and width in render-frames.md or not 🤔. Thank you as always, for being so supportive! 🤠

Not necessary, since you can already pass height and width using config.height and config.width as before :)

Thank you!

[exitflynn]

😳 😳
alrighty, @JonnyBurger! 🤠 I suppose the PR should be ready for review then! 😁

Please review and let me know if there's anything more to do here on the issue!

[JonnyBurger]

Nice! Everything works 🥳 Had to update the docs because a few options had the wrong name, but generally really nice work! Congrats on your first contribution and your bounty! 🥳 💰

Appreciate it and hope you had a good experience 😃

[exitflynn]

🥳 Thank you so much, @JonnyBurger for all the support! I remember hearing someone say that "the best thing about open source is the community" and I have time and again said the same thing about you in our Open-Source dev community's discord.
I definitely feel like i've learned quite a bit and thanks a lot for making the process less intimidating! I'll keep an eye out for other remotion issues in the near future and definitely hope to one day become a maintainer like you 😇
Thanks again for the guidance!

[JonnyBurger]

@exitflynn Thanks a lot! 🚀 🚀 🚀

[JonnyBurger]

@exitflynn For statistics, can you also estimate how long you spent on this PR in total? 😊

[exitflynn]

@JonnyBurger let's see, I actually started working on the issue on day 5 of being assigned (hectic week), spent a day or two to setup the project and get a basic feel of how React works. Including all the debugging (spent a lot of time on the is declared but its value is never read which i now realise is something they added in newer versions in React and I could have just ignored it all along) and troubleshooting git issues, I'd say all in all it must've taken me a week.

[JonnyBurger]

@exitflynn Okay, that is quite an outlier but I'll write you down as 35 hours then, hope that is seems about right! Thanks a lot for your effort!