-
Notifications
You must be signed in to change notification settings - Fork 161
Create videos from the API #378
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
Hello! Overall, quite good! However, for code extendability, a slightly different approach is required. I try to keep the code_generation_funnel as separated from the API as possible. I will give some specific things to look for using code review. This functionality is a great addition, I like that it does not break anything. Also a question: is it possible to later make the code support strings for model selection? Like, could we have a parameter that could be either int or a string? If not, do you think it would be a reasonable thing to do, to have it as string from the beginning and later add support for model names? Not sure if anybody wants to add it (would certainly be welcome), but just would be nice to work around this potential backward-compatibility issue. |
Well, if you wanted I can create a specific endpoint for this functionality like /depth/generate/video and do not use the code_generation_funnel, but implement a separated function for that endpoint. About this question "is it possible to later make the code support strings for model selection? " I do not understand what do you refer. Could you explain with more details and examples, please? 🤔 Thank you! |
Ah, I suppose you meant this: To be available inside the API and pass a string of the model name or pass an integer that matches that model, it that right? Like ? |
So basically depthmap_api.py should "wrap around" the core and ask it to do stuff, and then transform this stuff for API-specific needs. But in this state of MR, its leaking a bit - core is "aware" of the API stuff happening. |
Exactly. Sorry, did not sleep well tonight 🥶 |
| if you wanted I can create a specific endpoint for this functionality like /depth/generate/video |
Yes, I am aware of that. I just followed the order of what was already programmed. It is clear that the API should wrap around the core and not be mixed into it. Although I don't know the project enough to make such big changes. So I continued in the order in which things were already done. I think restructuring the API and refactoring it requires another separate pull request and I don't know if I would be able to do it without very clear instructions on how you would want it. |
I feel the same way, that a big task. Then just please try to make the core_generation_funnel not call the API specific code and then I think we can call it a day. Parameter names, optimal design decisions, et cetera can wait I suppose. |
Absoulutely! :D You have a much better vision than me. If you want to do it, it will be appreciated. |
I have tried to separate the code from lines 345 to 353 into a function outside of the core_generation_funnel. Although I haven't found a clean way to do it. In any case, I need variables set in the core_funnel to process the video later. So, the only way I found is to yield that variables instead of calling run_makevideo_api. And in another part of the code call run_makevideo_api. But I think is not a good approach.. Please, could you suggest me to abstract that part into another function, so that the core doesn't get mixed up with the API tasks? Thank you! |
You might be able to use a similar method to It does also feel like the second part of Great work! |
Ok, I'll do it this way. As soon as programmed I'll let you know. Thanks! |
I made several changes.
So, the core is not mixed with the api anymore when generating videos from the api. What do you think about this changes? Thank you! David Martin Rius |
Hello again :) I did not really get in to all the details of the code, but overall (at least architecturally), it looks very, very good 👍 |
This commit adds a new API endpoint for generating depth maps from input images in a video format. The endpoint supports various depth model options, including different pre-trained models. It also validates and processes video parameters such as number of frames, frames per second, trajectory, shift, border, dolly, format, and super-sampling anti-aliasing. The commit includes error handling for missing input images, invalid model types, and required video parameters. Additionally, it checks if a mesh file already exists, and if not, it generates a new one. The generated mesh is then used to create a depth video based on the specified parameters. See more information in the pull request description.
Indeed, very good code! Merging with next to no modifications. There are some security risks from exposing this functionality in API, but since we never advertised the API as something that can be made accessible from the internet, this is ok. Thank you so much for contributing this code ❤️ |
Hi @semjon00 !! I am very glad to contribute. Thank you so much for merging the code.
Do you know the critical points when generating a 3D mesh? I mean that ones that specially slow down the process and could use the gpu to speed up the process. If you had to do it where would you start? Thank you! David Martin Rius |
Honestly, I am not even exactly sure what magic happens over there. @thygate is the person who added this into the script - I think he borrowed this code from somewhere else - he knows how it works better than me. The proper way to change the code would be to find the upstream and contribute there (granted, hopefully that repository is still maintained), and then tweak this repository to use the newest upstream version. |
I think that code may come from https://github.com/vt-vl-lab/3d-photo-inpainting and also facebookresearch. Unfortunately, the last updates are from 3 years ago. So, this is the most updated repository altough it is almost a copy&paste of that project if I am not wrong. Maybe @thygate has an idea on how to do it. Thank you! |
Right, I remember now. Indeed, then the most reasonable thing to do is to just patch it here. After the fact I/we might try to find other "forks" and MR the changes there. |
Hello!
How are you doing? 😊 My name is David Martin Rius and I added a new functionality to create a video from an image with the API.
It does not affect the current extension behaviour. It is just an enhancement.
The endpoint is the same, /depth/generate I added a new option "RUN_MAKEVIDEO_API" in common_constants.py (available when calling /depth/get_options)
If this code is ok I will create a new endpoint in https://github.com/mix1009/sdwebuiapi to integrate it. If you don't know what is this other project, it is by now the most advanced and open source API for automatic1111.
This object "video_parameters" has this properties:
Important:
By now the output filename will have an extra underscore. For example if passed /my/folder/video.mp4 the ouput will be /my/folder/video.mp4_
The property "mesh_fi_filename" is optional, Is not needed. Also can be None or a path of a .obj file.
If you already have an .obj file it will create a video much faster. Creating a mesh is the lowest part of the process, so I recommend using it if you are rendering multiple videos.
On the other hand, if any of this properties is required and not passed is controlled by an exception inside run_makevideo_api function and will tell you what is missing.
The function run_makevideo_api can be found in scr/core.py
You can use this snippet example to create a video from the API:
Thank you! Enjoy it! 😊
David Martin Rius