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

Deal with double frames in animation, detect scenes, some small improvements. #120

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

edgarsi
Copy link

@edgarsi edgarsi commented Nov 16, 2020

No description provided.

ffmpeg select(n indexes from 0, Colab arguments from 1.
Claymation often have models moving only every second frame.
Interpolating several frames apart can smooth out even those. Often it's
askign too much however, since multiple frames apart is not what the net
is trained on. Only implemented for framerate increases 2x the original.
Copy link
Contributor

@AlphaGit AlphaGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Consider updating the version number, so that whenever people are reporting issues with the Colab file, they can tell easily if it's the latest version.

Also consider adding yourself to the credits, if you are ok with people reaching out to you. :)

I see you did a major work on that colab_interpolate file -- thank you very much for that! It was in need of some love.

And you added two new interpolation algorithms, wow! <3

colab_interpolate.py Outdated Show resolved Hide resolved
my_package/compiler_args.py Show resolved Hide resolved
@@ -94,6 +93,10 @@
"#@markdown A path, relative to your GDrive root, where you want the generated frame.\n",
"FRAME_OUTPUT_DIR = '/content/DAIN/output_frames' #@param{type:\"string\"}\n",
"\n",
"#@markdown ## Frame mixer\n",
"#@markdown Normally we interpolate between two adjacent frames. You can try to experiment with other mixers. Claymation mixer can give smoother outputs when animation only moves every second frame at times.\n",
"FRAME_MIXER = \"normal\" #@param [\"normal\", \"claymation\"] {type:\"string\"}\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think greased is missing here:

Suggested change
"FRAME_MIXER = \"normal\" #@param [\"normal\", \"claymation\"] {type:\"string\"}\n",
"FRAME_MIXER = \"normal\" #@param [\"normal\", \"claymation\", \"greased\"] {type:\"string\"}\n",

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found greased to be useless, its results disappointing. Didn't want to pollute the colab API with that. It may be best to remove it completely, but the maintenance burden is small, so left it in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me! I'm glad it wasn't a mistake.

@ynshung
Copy link

ynshung commented Nov 26, 2020

I ran this in Google Colab and it seems like it is slower (~4.75s/it) than the original (~3s/frame). Any possible reason why?

Edit: I think I'll do further testing in the future, it might be because I got a slower GPU while testing.

@iBobbyTS
Copy link

I ran this in Google Colab and it seems like it is slower (~4.75s/it) than the original (~3s/frame). Any possible reason why?

Edit: I think I'll do further testing in the future, it might be because I got a slower GPU while testing.

You can use nvidia-smi to check which GPU you got, I think in the Colab_DAIN.ipynb, there is a cell in the beginning that has !nvidia-smi in it, use this to check. Computing power(speed): V100>P100>T4>P4>K80
By the way, check my repo iBobbyTS/VFIN, it has DAIN and Super-SloMo in it, the usage is easier than Colab_DAIN.ipynb, in yesterday's release 1.2, the speed of DAIN improved a lot, I deleted some useless code and saved huge amount of time.

@VelocityRa
Copy link

Is there a reason this hasn't been merged?

@AlphaGit
Copy link
Contributor

I have incorporated these changes into my fork of this repository.

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

Successfully merging this pull request may close these issues.

5 participants