Skip to content
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

Possible optimizations in ASCII generation, video quality, and video playback #3

Open
karoush1 opened this issue Apr 18, 2021 · 1 comment

Comments

@karoush1
Copy link

karoush1 commented Apr 18, 2021

Hi, I saw the code on touhou_bad_apple_v2.py and I though of some possible speed improvements right off the bat.

First is ASCII generation. As you're doing right now, the best way to improve processing speed is by creating a Pool and allocating each frame to a worker until the frame extraction is over. The disadvantage is that you lose the current progress bar you have implemented, and instead will need something more complex like an N lines progress bar, one for each worker in the pool, or a signaling method between threads, like a semaphore, and is just too much work for what the current project is aiming for. You can also improve the video quality by adding more characters to the list, more specifically a space(all black) " " and a block character(all white) "\u2588", and adapting the padding calculation on line 86 to include the new characters.

Another improvement, and probably the most important one, is saving the ASCII frames to a single file containing all frames, instead of multiple files. The reasoning is that opening and closing files takes time, which would be better spent synchronizing the frames in the terminal.

Checking files also takes time. As of right now, you are manually checking every single .txt file for all the frames and checking if they exist. This can be done much faster if you use instead os.listdir() and filter the results containing the string .txt, then count the results(or a single file if you use the approach above). Also, please always use os.path.join() instead of manually using /. Reasoning is that the os.path library provides cross-platform separators and better handling of absolute paths, which is much less error prone.

I could make a fork and do those changes, but I think it would be very useful if you did them yourself, since you can learn new skills and functions with them. Tell me what you think about my suggestions, then close this issue later.

Edit: my mistake, I just noticed you changed to a white background in the video. Correct characters are a space(all white) " " and a block character(all black) "\u2588"

@CalvinLoke
Copy link
Owner

Hi there, really appreciate the effort looking through my code.

It was originally a dumb weekend project that I had in mind and wasn't really expecting it to be getting to much attention. It was more for me to put what I have learnt of python so far.

I'm still very fuzzy on the concept of multi threading, though it is definitely something that I should take a look as part of my learning process.

Now that you mention it, yeah I guess file I/O is probably the major bottleneck right now which might be the main reason of the desynchronization issues.

I'll definitely rectify the os.path issue as I do plan to run this on different environments to test the performance.

I'll keep this issue open until I make the changes.

Once again really appreciate the time and effort for looking through code that started out as a side gig.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants