Skip to content

Reddit Post Position Config & Download Progress Bar #693

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

Conversation

William9923
Copy link
Collaborator

@William9923 William9923 commented Jun 25, 2022

Description

Added:

  • Dynamic image clip position for each available background
  • Progress bar when downloading youtube video
  • Few additional background video (GTA, Rocket League, Assasin Creed) as example

Why? (Motivation)

Although the bot has produced a lot of good videos with minecraft background, I noticed that on some other background, the image of reddit post was blocking the background (check below image). So, it inspire me to make different configuration for each background. For now, it would be the reddit post image position on the background

before after

Before

After

Also, when trying out different background, I noticed that we need to download all background video simultaneously. Because the time it takes to download all video is quite long, I propose that the bot only need to download for needed video & add the progress bar to track the download progress.

During End

Downloading...

Done

How (?)

For changing the position of reddit post image for each background, I use the available set_position method in moviepy. After refactoring the code for the background video part, we can further make separate configuration (not only the position of reddit post image), such as the blur effect (opacity) etc.

For the progress bar, I simply add tqdm progress bar and use on_progress_callback that was available from pytube API when downloading the video.

Test Guide

To make testing the new feature easier, you could follow this step by step guides below (i don't include other functionality test, but it should be the same as other functionality):

  1. Testing out different screenshot placement in background.
  • First, might need to read this. It contains reference on how moviepy put the image in the background clips
  • Second, try out different background to see the diff. (example: minecraft use center , while other like GTA Stunt race move the screenshot slightly higher)
  • Third, feel free to modify the position for each background.

  1. Testing out the new progress bar during Youtube video download
  • First, run the program with background that you never downloaded (or just remove the background vids)
  • Second, wait till the download done. You will see the progress bar during download...
  1. Testing out new random background (default, if not filled or unsupported background)
  • Don't fill the .env variable BackgroundChoice. It will use random video (from supported vids) as background
  1. Adding new background (this is not new feature, but this PR slightly alter how to add new video as background)

@William9923 William9923 marked this pull request as draft June 25, 2022 04:36
@William9923 William9923 marked this pull request as ready for review June 25, 2022 04:38
@William9923 William9923 changed the title Comment (ImageClip) Position Configuration & Download Progress Bar Reddit Post Position Config & Download Progress Bar Jun 25, 2022
@ghost
Copy link

ghost commented Jun 25, 2022

Description

Added:

  • Dynamic image clip position for each available background
  • Progress bar when downloading youtube video
  • Few additional background video (GTA, Rocket League, Assasin Creed) as example

Why? (Motivation)

Although the bot has produced a lot of good videos with minecraft background, I noticed that on some other background, the image of reddit post was blocking the background (check below image). So, it inspire me to make different configuration for each background. For now, it would be the reddit post image position on the background

before after
Before

After

Also, when trying out different background, I noticed that we need to download all background video simultaneously. Because the time it takes to download all video is quite long, I propose that the bot only need to download for needed video & add the progress bar to track the download progress.

During End
Downloading...

Done

How (?)

For changing the position of reddit post image for each background, I use the available set_position method in moviepy. After refactoring the code for the background video part, we can further make separate configuration (not only the position of reddit post image), such as the blur effect (opacity) etc.

For the progress bar, I simply add tqdm progress bar and use on_progress_callback that was available from pytube API when downloading the video.

Hello,

I'm not a developer, but I've been using this project for a week, and I try to help at least with ideas and tests, among other basic things.

I was reading your PR and just this week I was thinking about this improvement.

I thought maybe you could see the following:

Find out what the official TikTok "safe zones" are, and it would be great to be able to move the screenshot also horizontally, since currently, as the TikTok interface is published, it covers part of the Screenshot, I leave you an example:
289022985_722387272427917_353594956416899694_n

On the official website of tiktok, there are the rules of the safezones so that your content is not covered by its tiktok interface:
https://ads.tiktok.com/help/article?aid=10002742

basically there is 120px on the right side that contains buttons from the tiktok interface, it would be great if the screenshots respect that 120px on the right side

thumbnail_e9dd6157a46442e69b5ea15f28d24ab5_jpg

I hope I didn't bother you with this, but your PR seemed the perfect thing to solve this detail too

@lamasterso
Copy link

@IslaFTW Bro Do you know how to make the text size a little bit larger ?

@ghost
Copy link

ghost commented Jun 25, 2022

@IslaFTW Bro Do you know how to make the text size a little bit larger ?

I think it is not possible, but it would be great to be able to control the size of the font and also the type of Font, for example the "Ubuntu" font in size 12, it is very pleasant to read, and it is very beautiful

@lamasterso
Copy link

I tried to resize the screenshot and did everything but there is no variable to control the Height Of the screenshot, Therefore I was only able to manage changing the width, But I think making the height of the screenshot more will stretch the font somehow.

Now the font is very wide but in very small letters to read especially the comments.

@William9923
Copy link
Collaborator Author

William9923 commented Jun 25, 2022

@IslaFTW One of the solution that I can think of is to check & resizing the image to ensure that the image respect the Tiktok interface (120px), like what @LamasterO did . I will try to see what I can do on this. Thank you for your suggestion 👍 .

While moving the image entirely to the left to avoid the tiktok interface could be a solution, I have tried it and it seems weird (?), ex:
image

I think this is a great idea to open up as an issue too, as I notice some of the bigger image clip being covered up the button on Youtube short.

@JasonLovesDoggo
Copy link
Collaborator

This is great and all but you are by far over complicating things, you don't need separate files and classes for every single background and you don't need a background swapper class

Also @IslaFTW suggestion was great

I will merge this into a separate branch and improve this there in 3 days

@JasonLovesDoggo JasonLovesDoggo added after next release PR is ready to merged after next release enhancement New feature or request labels Jun 26, 2022
@William9923 William9923 force-pushed the feat/background-configs branch from 0f82738 to a81b0c3 Compare June 29, 2022 16:32
@William9923
Copy link
Collaborator Author

Hi @JasonLovesDoggo , I have adjusted this PR with the recent code change and refactor background.py for downloading background video from Youtube parts to simplify it. Would you be kindly enough to review the changes again. Thank you 🙏

@JasonLovesDoggo
Copy link
Collaborator

Hi @JasonLovesDoggo , I have adjusted this PR with the recent code change and refactor background.py for downloading background video from Youtube parts to simplify it. Would you be kindly enough to review the changes again. Thank you 🙏

I will look at it when I get home around
6:00 p.m. EST

@CJthisis
Copy link
Contributor

CJthisis commented Jul 1, 2022

Hey there, I tried to run the code in combination with the latest develop build, but it doesn't work.

Specifically, when trying to download the background videos, in the line 19 of background.py, there is is the name 'Progress', which does't seem to appear anywhere else in the file?
I don't know how to fix this, maybe you know what to do?

grafik

@JasonLovesDoggo
Copy link
Collaborator

Hey there, I tried to run the code in combination with the latest develop build, but it doesn't work.

Specifically, when trying to download the background videos, in the line 19 of background.py, there is is the name 'Progress', which does't seem to appear anywhere else in the file?
I don't know how to fix this, maybe you know what to do?

grafik

I will take a look at that

@William9923

This comment was marked as duplicate.

@William9923
Copy link
Collaborator Author

William9923 commented Jul 2, 2022

Hey there, I tried to run the code in combination with the latest develop build, but it doesn't work.

Specifically, when trying to download the background videos, in the line 19 of background.py, there is is the name 'Progress', which does't seem to appear anywhere else in the file? I don't know how to fix this, maybe you know what to do?

grafik

I will help to look into it too. Maybe it related on the new progress bar that were included in this PR. Also need to adjust to new changes in develop branch haha 🤣

@William9923 William9923 closed this Jul 2, 2022
@William9923 William9923 reopened this Jul 2, 2022
@William9923 William9923 force-pushed the feat/background-configs branch 2 times, most recently from 8e4d813 to c21b5d8 Compare July 2, 2022 09:03
@William9923
Copy link
Collaborator Author

William9923 commented Jul 2, 2022

Hi @CJthisis and @JasonLovesDoggo , I have updated my code to the newest develop branch (and tested it!). The problem from your screenshot should not happen anymore, and would you kindly enough to try it again? Thank youu 🙏 .

@William9923 William9923 mentioned this pull request Jul 2, 2022
@JasonLovesDoggo
Copy link
Collaborator

This is great but I just have one last feature request from this Have it so that if the background choice option is empty It will pick a random background

@CJthisis
Copy link
Contributor

CJthisis commented Jul 2, 2022

This is great but I just have one last feature request from this Have it so that if the background choice option is empty It will pick a random background

This would be great!

@CJthisis
Copy link
Contributor

CJthisis commented Jul 2, 2022

Hi @CJthisis and @JasonLovesDoggo , I have updated my code to the newest develop branch (and tested it!). The problem from your screenshot should not happen anymore, and would you kindly enough to try it again? Thank youu 🙏 .

I will do so tomorrow when I get home. Do you have updated your fork too? If so, I would use that version

@William9923
Copy link
Collaborator Author

William9923 commented Jul 2, 2022

This is great but I just have one last feature request from this Have it so that if the background choice option is empty It will pick a random background

Will try to implement it, thanks for the suggestion

@William9923
Copy link
Collaborator Author

William9923 commented Jul 2, 2022

Hi, I've implemented the default (if user not pick any background) background == random background (that the bot support). I will write a guide to test it out so you guys have an easier time trying the new feature. Thanks 🙏

edit:
To try out the new feature, please refer here. Thank youu for your time @JasonLovesDoggo @CJthisis 🙏

@JasonLovesDoggo JasonLovesDoggo added before next release Issue must be solved / PR must be merged before next release and removed after next release PR is ready to merged after next release labels Jul 3, 2022
@CordlessCoder
Copy link
Collaborator

Will test and review when I get home

@callumio callumio added this to the 2.3 milestone Jul 3, 2022
@William9923 William9923 force-pushed the feat/background-configs branch 2 times, most recently from 5c248bc to 836dcf6 Compare July 4, 2022 00:50
@JasonLovesDoggo
Copy link
Collaborator

@CordlessCoder If this works for you. Merge it

@callumio callumio linked an issue Jul 4, 2022 that may be closed by this pull request
@William9923 William9923 force-pushed the feat/background-configs branch from 836dcf6 to 59d07f5 Compare July 5, 2022 04:04
@William9923
Copy link
Collaborator Author

@CordlessCoder hi, due to new update in develop branch, I need to adjust a few line of codes. Please update your branch (or just delete and pull it again haha) when reviewing. Thankss 🙏

@CJthisis
Copy link
Contributor

CJthisis commented Jul 5, 2022

Hey there, in final_video.py, it keeps giving me this SyntaxError: '(' was never closed. I am not experienced enough to fix this by myself, where does the missing ')' go? @William9923

grafik

@William9923
Copy link
Collaborator Author

William9923 commented Jul 5, 2022

Hey there, in final_video.py, it keeps giving me this SyntaxError: '(' was never closed. I am not experienced enough to fix this by myself, where does the missing ')' go? @William9923

grafik

hi @CordlessCoder , a little bit confused tho, but the code in my fork should only have 122 lines (in final_video.py)? Mm, r u having merge conflict when pulling it (because i often rebase it to update to develop branch)?
image

ps: If you having difficulity merging it, best option is to delete the branch & simply use whatever in the remote 🙏 . It will automaticly use the latest. Thx

@William9923
Copy link
Collaborator Author

bump @JasonLovesDoggo , @CordlessCoder any update on this PR 😄 ? A little bit tired of updating it to the newest develop branch every day haha.

@JasonLovesDoggo
Copy link
Collaborator

The thing is we are about to change from the ENV file to a different system so it would be incompatible

@callumio
Copy link
Collaborator

callumio commented Jul 6, 2022

bump @JasonLovesDoggo , @CordlessCoder any update on this PR smile ? A little bit tired of updating it to the newest develop branch every day haha.

Once the new config system is in, I'll fix the merge conflicts and test

@William9923
Copy link
Collaborator Author

The thing is we are about to change from the ENV file to a different system so it would be incompatible

I see, thx for the heads up.

@William9923
Copy link
Collaborator Author

bump @JasonLovesDoggo , @CordlessCoder any update on this PR smile ? A little bit tired of updating it to the newest develop branch every day haha.

Once the new config system is in, I'll fix the merge conflicts and test

sure", thx btw. I'm always available incase any issue comes up when merging 🙏 .

@JasonLovesDoggo JasonLovesDoggo changed the base branch from develop to feat/RedditPostPosition July 7, 2022 02:16
JasonLovesDoggo added a commit that referenced this pull request Jul 7, 2022
@JasonLovesDoggo JasonLovesDoggo merged commit 73d15fe into elebumm:feat/RedditPostPosition Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
before next release Issue must be solved / PR must be merged before next release enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: make image position configurable
6 participants