Unified ffmpeg wrapper class.
Closed, ResolvedPublic

Description

Right now we have multiple pieces of code (animation exporter, video importer, and the recorder docker) that all use FFMPeg, but use separate interfaces/code paths to do so. This obviously isn't sustainable and it can lead to a lot of inconsistencies down the road, so we need to start thinking about building an ffmpeg wrapper that is flexible and reusable enough to use across Krita's tools, present and future.

Note: We may also want to take a look at the UX/UI for each of these pieces and see if they can also be unified/homogenized/standardized in some way. The most dated of relevant GUI is probably the animation exporter, which could probably use a bit of an upgrade anyway, to keep up with some of the nice features of the newer dialogs.

What I would like to see is FFMpeg added to the settings under the Configure Krita. And any feature they use would point them at the settings page to add it in 1 central place.

Things that should be on the settings page:

Let user pick multiple versions of ffmpeg that would get added to the drop down - This would let them manage their built in ffmpeg that might come with their distribution, latest stable ffmpeg, or the latest test version if they really really need the latest format (say AV1)

Let them pick which one they want to use by default - self explanatory

A grid list of capabilities - This would let the user see which formats he can use or can't use. So there would be no surprises like their open source patent free version has no h264/h265 and etc.

A hyperlink to a webpage with instructions on how to get ffmpeg - One of the big issues people have with ffmpeg is that it is too confusing for people on where or how to get it. We would check their OS and send them to a simple guide that should be simple enough for even a fresh windows user can follow.

Storing the settings data and user interaction

FFMpeg user settings - We should probably have a standard settings also for all forms of encoding, if an FFMpeg version does not support those options, they should either be not there, or maybe in the case of common options, grayed out with a warning. We should probably also record the recent settings, and let people pick favorites.

Decide what to expose to the user - Technically speaking, we can expose the entire FFMpeg settings to the user. "ffmpeg -h full" pretty much offers a parse-able breakdown of available options. In theory, we can automatically parse that and create a gui of rich options a user can pick. Or we can pre-parse them on our end automatically and just bundle them as ui files.

Storing the capability data - If we do record FFMpeg's capabilities either mentioned above or even more simply just the available formats, do we parse the FFMpeg capabilities every time? Or do we store that data somewhere? Then say check if something changed via modified date? and only reparse then.

And finally we get to the FFMpeg wrapper!

The reason I bring up all of the stuff above is we need to have a vision of what we plan to do with FFMpeg in the long term so that we can decide how to proceed with making a universal wrapper that isn't only useful now but also useful in the future. Obviously we can't plan for everything but at the very least we can get a grasp of what we may or may not need and make it flexible enough while following best practices.

The first thing to note is do we bundle things like find FFMpeg or test it's capabilities in the wrapper? Or do we store common tools like those in a separate class?

The second thing we need to decide is how to handle output. KisVideoSaver chooses to put progress in the temp folder, while log data in the same folder as the file where it goes with mergechannels. In comparison, recorder ffmpeg wrapper reads the buffer and gets progress from there. It reads the data based on new lines.

Now then,

  1. should the wrapper always log? let the developer decide? or make it an optional setting for the user? if so, what path should the logs be?
  1. For progress, should it be put into temporary? Or should it just be piped into stderr and read from there?
  1. Do we include the progress dialog in the wrapper? Or do we let the caller class handle it?
  1. Do we include static functions for things that don't need progress or leave that up to the caller class? (like binary data, information and etc)
  1. What should be the best practices for when we choose to store or to pipe things, to create files, and to do it where? Temp folder? local folder?

I am probably missing a few other things but it's already getting late so I'll leave it here for debate.

Lastly, I wanted to say that I tried making KisFFMpegWrapper to centralize the use cases of both KisVideoSaver and RecorderFFMpegWrapper. I also started a little early work on capabilities checking but didn't know whether to proceed to store that info so for now I only do a basic check every time it is loaded.

My goal is to first start with implementing the settings, then remove KisVideoSaver and RecorderFFMpegWrapper and replace it with KisFFMpegWrapper. But if someone has a better approach, suggestions or just want to give their 2 cents, I am all ears.

tymond added a subscriber: tymond.Apr 5 2021, 4:03 PM

A hyperlink to a webpage with instructions on how to get ffmpeg - One of the big issues people have with ffmpeg is that it is too confusing for people on where or how to get it. We would check their OS and send them to a simple guide that should be simple enough for even a fresh windows user can follow.

It should link to here: https://docs.krita.org/en/reference_manual/render_animation.html#setting-up-krita-for-exporting-animations - and if anyone wants to improve on the documentation, the repository is open for MRs ;)

A hyperlink to a webpage with instructions on how to get ffmpeg - One of the big issues people have with ffmpeg is that it is too confusing for people on where or how to get it. We would check their OS and send them to a simple guide that should be simple enough for even a fresh windows user can follow.

It should link to here: https://docs.krita.org/en/reference_manual/render_animation.html#setting-up-krita-for-exporting-animations - and if anyone wants to improve on the documentation, the repository is open for MRs ;)

The documentation says the reason ffmpeg is not included is to save space, is that the actual reason? Cause if that is the case, we can make it even simpler and download ffmpeg for their platform on demand with 1 click.

Otherwise, yeah the guide needs to be fixed up. Each OS needs its own steps instead of jumping between platforms every step.

eoinoneill added a comment.EditedApr 5 2021, 7:26 PM

The documentation says the reason ffmpeg is not included is to save space, is that the actual reason?

No, this is not the actual reason as of today (it may have been at the time the documentation was written, though...)

We looked into bundling FFMPEG but, due to licensing concerns we think it will be simpler to ask people to get their own version of FFMPEG. Otherwise, Krita would come with a version of FFMPEG that doesn't support all of the desired file formats.

I agree with having better documentation for how to install and setup FFMPEG for Krita. We'll have to get on that after unifying the FFMPEG wrappers (which I'm looking into right now.)

The documentation says the reason ffmpeg is not included is to save space, is that the actual reason?

No, this is not the actual reason as of today (it may have been at the time the documentation was written, though...)

We looked into bundling FFMPEG but, due to licensing concerns we think it will be simpler to ask people to get their own version of FFMPEG. Otherwise, Krita would come with a version of FFMPEG that doesn't support all of the desired file formats.

Yeah, pretty much h264/h265 would be out the door. Okay, just checking so we are all on the same page.

I agree with having better documentation for how to install and setup FFMPEG for Krita. We'll have to get on that after unifying the FFMPEG wrappers (which I'm looking into right now.)

Yeah, documentation is the last step.

In theory, the wrapper I made already unifies most if not all of the functions of the 2 other wrappers and more. The question comes down to where the line is drawn for what should be part of the wrapper or should be seperate. Take KisSaver, it has 2 classes, 1 for the wrapper and 1 for prebuilt functions that talk with the wrapper. So should the wrapper be barebone and then have separate classes for interacting with the wrapper? Or stick all of them together?

What I would like to see is FFMpeg added to the settings under the Configure Krita. And any feature they use would point them at the settings page to add it in 1 central place.

Yeah, I think we can all agree with having a central spot for ffmpeg in the configuration.
It might have made sense when there was only one client, but now that we have the animation renderer, video importer and recorder all using ffmpeg, it really belongs in the general Krita configuration.

This is definitely priority 1.

Let user pick multiple versions of ffmpeg that would get added to the drop down - This would let them manage their built in ffmpeg that might come with their distribution, latest stable ffmpeg, or the latest test version if they really really need the latest format (say AV1)

Let them pick which one they want to use by default - self explanatory

A grid list of capabilities - This would let the user see which formats he can use or can't use. So there would be no surprises like their open source patent free version has no h264/h265 and etc.

As for allowing the user to configure multiple versions of ffmpeg with a grid of capabilities, that sounds nice, if a bit complex for Krita's current use case.
That's not to say I'm opposed to it, but I'd prefer to hold off on that until we have the basics in place. Minimum viable product and all that. :)

Unless there is some reason why we need this right away, I'll call this priority 2.

A hyperlink to a webpage with instructions on how to get ffmpeg - One of the big issues people have with ffmpeg is that it is too confusing for people on where or how to get it. We would check their OS and send them to a simple guide that should be simple enough for even a fresh windows user can follow.

Yeah. I totally agree with this.

I somewhat recently added a link to the documentation on setting up Krita with ffmpeg (like @tymond suggested above) to the error dialog that pops up when ffmpeg can't be found for animation rendering.

We probably want to make sure all of that documentation is still up to date.

Storing the settings data and user interaction

FFMpeg user settings - We should probably have a standard settings also for all forms of encoding, if an FFMpeg version does not support those options, they should either be not there, or maybe in the case of common options, grayed out with a warning. We should probably also record the recent settings, and let people pick favorites.

Decide what to expose to the user - Technically speaking, we can expose the entire FFMpeg settings to the user. "ffmpeg -h full" pretty much offers a parse-able breakdown of available options. In theory, we can automatically parse that and create a gui of rich options a user can pick. Or we can pre-parse them on our end automatically and just bundle them as ui files.

Storing the capability data - If we do record FFMpeg's capabilities either mentioned above or even more simply just the available formats, do we parse the FFMpeg capabilities every time? Or do we store that data somewhere? Then say check if something changed via modified date? and only reparse then.

Right now I really like how the Recorder Docker handles settings: all of the most fundamental settings are given widgets, and then the value of those widgets are injected into a template string "profile" of ffmpeg arguments, which can also be optionally edited by the user. Personally, I think that this GUI hits the right balance of ease-of-use and flexibility. It gives new users some relatively straight-forward settings, while also giving advanced users direct access to the ffmpeg argument to do just about anything they want.

I'm not sure how well that idea scales or how well it could be applied across animation rendering or video importing, but it feels a lot cleaner and more powerful than our old Render Animation dialog.
The current render animation dialog is just overflowing with widgets, and feels a bit less user-friendly than the recorder settings or video import settings windows.

No matter what we do on the UI/UX side I'd like us to try to find a nice balance between simplicity and flexibility, since we want Krita to be a tool for artists of all levels. :)

Frontend changes are kind of priority... 1.5? I feel the video import and recorder UX is pretty nice right now.
I think the render animation dialog could use some work, but what's more important right now is making sure that everything can use the same backend.

And finally we get to the FFMpeg wrapper!

The reason I bring up all of the stuff above is we need to have a vision of what we plan to do with FFMpeg in the long term so that we can decide how to proceed with making a universal wrapper that isn't only useful now but also useful in the future. Obviously we can't plan for everything but at the very least we can get a grasp of what we may or may not need and make it flexible enough while following best practices.

The first thing to note is do we bundle things like find FFMpeg or test it's capabilities in the wrapper? Or do we store common tools like those in a separate class?

I don't know for sure, but my first instinct is to put just about everything FFMpeg-related input a single wrapper class.
The most important part is making it accessible to reusable across all of our existing features, and future ones too as much as we realistically can.

I'll admit I don't know a ton about ffmpeg so I could be wrong, but it seems like what we need to do isn't complex enough that we need a big tree of classes or anything.
Any utility-type stuff can be done with static methods, too.

The second thing we need to decide is how to handle output. KisVideoSaver chooses to put progress in the temp folder, while log data in the same folder as the file where it goes with mergechannels. In comparison, recorder ffmpeg wrapper reads the buffer and gets progress from there. It reads the data based on new lines.

1.should the wrapper always log? let the developer decide? or make it an optional setting for the user? if so, what path should the logs be?
 
2. For progress, should it be put into temporary? Or should it just be piped into stderr and read from there?
 
3. Do we include the progress dialog in the wrapper? Or do we let the caller class handle it?
 
4. Do we include static functions for things that don't need progress or leave that up to the caller class? (like binary data, information and etc)
 
5. What should be the best practices for when we choose to store or to pipe things, to create files, and to do it where? Temp folder? local folder?

These are good questions and I'm not really sure I have a great answer to any of them right now. What do you think?

I think ideally any "working data" and logs should be stored in temp, so that from the average users standpoint it's all kind of behind the scenes and cleaned up for them.
From the artists perspective, it'd be nice if they hit render/export/etc., and get whatever output they want (a video file, an image sequence or both) and nothing more.
In the event that we/they want to take a peek at the logs or whatever, then they can pull those out of their temp.

I get a feeling you're much more familiar with ffmpeg than I am, so I'm curious to know what you think!
Eoin and I are taking some time this week to look into the ffmpeg situation more, so this post has given us me a lot to think about.

Lastly, I wanted to say that I tried making KisFFMpegWrapper to centralize the use cases of both KisVideoSaver and RecorderFFMpegWrapper. I also started a little early work on capabilities checking but didn't know whether to proceed to store that info so for now I only do a basic check every time it is loaded.

My goal is to first start with implementing the settings, then remove KisVideoSaver and RecorderFFMpegWrapper and replace it with KisFFMpegWrapper. But if someone has a better approach, suggestions or just want to give their 2 cents, I am all ears.

Great! Thanks!

Please feel free to submit an early WIP merge request, even if you're just poking around with some ideas.
That way we can all kind of stay in tuned with eachother on this. ;)

Let user pick multiple versions of ffmpeg that would get added to the drop down - This would let them manage their built in ffmpeg that might come with their distribution, latest stable ffmpeg, or the latest test version if they really really need the latest format (say AV1)

Let them pick which one they want to use by default - self explanatory

A grid list of capabilities - This would let the user see which formats he can use or can't use. So there would be no surprises like their open source patent free version has no h264/h265 and etc.

As for allowing the user to configure multiple versions of ffmpeg with a grid of capabilities, that sounds nice, if a bit complex for Krita's current use case.
That's not to say I'm opposed to it, but I'd prefer to hold off on that until we have the basics in place. Minimum viable product and all that. :)

Unless there is some reason why we need this right away, I'll call this priority 2.

Well it boils down to how much capabilities we wish to record, if we are talking about just the basics of encoding and formats, that should be simple. It only gets complicated if we dig deeper into advanced settings like gif transparency options or etc.

There is quite a bit of benefit recording the capabilities, outside of simply the obvious usage, for example. If Krita fails to open a file, ffmpeg can act as a backup to import it. This would also allow formats like gif to be opened directly via right click, open in Krita.

And once that data is recorded, the UX portion isn't much of an issue.

Of course I am all for one step at a time. Feature creep is a thing. And part of the reason a lot of stuff never get done or get over-engineered.

Storing the settings data and user interaction

FFMpeg user settings - We should probably have a standard settings also for all forms of encoding, if an FFMpeg version does not support those options, they should either be not there, or maybe in the case of common options, grayed out with a warning. We should probably also record the recent settings, and let people pick favorites.

Decide what to expose to the user - Technically speaking, we can expose the entire FFMpeg settings to the user. "ffmpeg -h full" pretty much offers a parse-able breakdown of available options. In theory, we can automatically parse that and create a gui of rich options a user can pick. Or we can pre-parse them on our end automatically and just bundle them as ui files.

Storing the capability data - If we do record FFMpeg's capabilities either mentioned above or even more simply just the available formats, do we parse the FFMpeg capabilities every time? Or do we store that data somewhere? Then say check if something changed via modified date? and only reparse then.

Right now I really like how the Recorder Docker handles settings: all of the most fundamental settings are given widgets, and then the value of those widgets are injected into a template string "profile" of ffmpeg arguments, which can also be optionally edited by the user. Personally, I think that this GUI hits the right balance of ease-of-use and flexibility. It gives new users some relatively straight-forward settings, while also giving advanced users direct access to the ffmpeg argument to do just about anything they want.

I'm not sure how well that idea scales or how well it could be applied across animation rendering or video importing, but it feels a lot cleaner and more powerful than our old Render Animation dialog.
The current render animation dialog is just overflowing with widgets, and feels a bit less user-friendly than the recorder settings or video import settings windows.

No matter what we do on the UI/UX side I'd like us to try to find a nice balance between simplicity and flexibility, since we want Krita to be a tool for artists of all levels. :)

Frontend changes are kind of priority... 1.5? I feel the video import and recorder UX is pretty nice right now.
I think the render animation dialog could use some work, but what's more important right now is making sure that everything can use the same backend.

The recorder settings interface is great for advanced users, but may be a bit tricky for the average windows user. For example, let us say I want to do some sort of compression. I would have to go look through the ffmpeg documentation. But as an advanced user, it is pretty much the best you can get. Only thing that could make it better is highlighting and maybe a bit better profile management. (but of course one step at a time)

For the average user, I was thinking something like that but generating the UI widgets dynamically based on their ffmpeg capabilities. And then the advanced option can be like ffmpeg recorder.

And yes, I totally agree with the animation export dialog. It really needs to be rewritten from scratch as it seems like things have been hacked in over time so a lot of bloat.

As far as sharing goes, the recorder and the export use the same functionality so they can pretty much share the same settings. The import would be different.

And finally we get to the FFMpeg wrapper!

The reason I bring up all of the stuff above is we need to have a vision of what we plan to do with FFMpeg in the long term so that we can decide how to proceed with making a universal wrapper that isn't only useful now but also useful in the future. Obviously we can't plan for everything but at the very least we can get a grasp of what we may or may not need and make it flexible enough while following best practices.

The first thing to note is do we bundle things like find FFMpeg or test it's capabilities in the wrapper? Or do we store common tools like those in a separate class?

I don't know for sure, but my first instinct is to put just about everything FFMpeg-related input a single wrapper class.
The most important part is making it accessible to reusable across all of our existing features, and future ones too as much as we realistically can.

I'll admit I don't know a ton about ffmpeg so I could be wrong, but it seems like what we need to do isn't complex enough that we need a big tree of classes or anything.
Any utility-type stuff can be done with static methods, too.

Okay, that is what I wanted to confirm.

I think ideally any "working data" and logs should be stored in temp, so that from the average users standpoint it's all kind of behind the scenes and cleaned up for them.
From the artists perspective, it'd be nice if they hit render/export/etc., and get whatever output they want (a video file, an image sequence or both) and nothing more.
In the event that we/they want to take a peek at the logs or whatever, then they can pull those out of their temp.

I get a feeling you're much more familiar with ffmpeg than I am, so I'm curious to know what you think!
Eoin and I are taking some time this week to look into the ffmpeg situation more, so this post has given us me a lot to think about.

Well, let me see if I can give a detailed account of where we stand with this and what I chose for my wrapper, maybe that will help

1.should the wrapper always log? let the developer decide? or make it an optional setting for the user? if so, what path should the logs be?

kisVideoSaver = always logs to same folder as the export location. The wrapper itself does take a custom path, but Export enforces it to same path.
RecorderFFMpegWrapper = Just records the error with no logging
KisFFMpegWrapper = Logging is done to central log, but you can also specify a log or hook onto the readready signal, Importer leaves it up to the wrapper.

My opinion: I am with you about not dropping log files in user directories. I think either the central log or work directory is a better way to go. Giving users an error is a good, but that won't help if Krita ends up crashing for some reason.

  1. For progress, should it be put into temporary? Or should it just be piped into stderr and read from there?

kisVideoSaver = creates a progress file in temporary folder
RecorderFFMpegWrapper = reads it via standard stderr output
KisFFMpegWrapper = forces it to stderr

My opinion: I am a bit undecided here. The part of me who wants to keep things centralized and simple prefers the stderr approach, the part of me who wants to account for every single scenario probably thinks its better to put it as temp file in event of a crash. I went with the stderr approach because I figured the chance of it happening when Krita is just sitting there is close to slim to none.

  1. Do we include the progress dialog in the wrapper? Or do we let the caller class handle it?

kisVideoSaver = Includes it in the wrapper, there is a batchmode option to disable it but I am not sure if its connected or not
RecorderFFMpegWrapper = lets caller class handles it
KisFFMpegWrapper = includes it in the wrapper, enabling batchmode by the developer would hide it

My opinion: I think a progress dialog should be default for when loading potentially long external processes, so it probably should be in the wrapper. If a developer wants to bypass the dialog or do their own, they can just enable batch mode.

  1. Do we include static functions for things that don't need progress or leave that up to the caller class? (like binary data, information and etc)

kisVideoSaver = Has no real way of returning any data outside of writing files
RecorderFFMpegWrapper = Same as above
KisFFMpegWrapper = Can retrieve data all at once or on readready, also includes static functions for quick getting of data without the extra overhead

My opinion: I think this has already been agreed on that we can include static methods for things like this

  1. What should be the best practices for when we choose to store or to pipe things, to create files, and to do it where? Temp folder? local folder?

Exporter = Pretty much enforces output directory to match where everything is put without any sub folders. Doesn't work with pipes and everything is file based
Recorder = Uses a work directory that is set by the user
Importer = Creates a work directory in the user path

My opinion: I think presetting a work directory that is set by default but can be changed by the user is probably the best approach (so yes, I am siding against my own approach for the importer and siding with the recorder implementation). It is a good balance between the user knowing where to find stuff in event of a crash, and not invading into the user space. This work directory should probably be in the ffmpeg settings for central access. The only thing is since it isn't a temp directory nor fully visible to the user, we would probably want to clean it up in a balanced way of the user being able to access it when needed (so data isn't lost if the user tries things back and forth), but at same time the user not realizing they are out of space a year later cause the directory is full.

I hope this helps make the final decision.

Lastly, I wanted to say that I tried making KisFFMpegWrapper to centralize the use cases of both KisVideoSaver and RecorderFFMpegWrapper. I also started a little early work on capabilities checking but didn't know whether to proceed to store that info so for now I only do a basic check every time it is loaded.

My goal is to first start with implementing the settings, then remove KisVideoSaver and RecorderFFMpegWrapper and replace it with KisFFMpegWrapper. But if someone has a better approach, suggestions or just want to give their 2 cents, I am all ears.

Great! Thanks!

Please feel free to submit an early WIP merge request, even if you're just poking around with some ideas.
That way we can all kind of stay in tuned with eachother on this. ;)

Alright, since I got the basic idea where people stand on some of the stuff, I'll make a WIP patch to show it just as soon as I finish my Krita Python plugin :)
(I'm effectively "emulating" an oncanvas text tool in a very very hacky way, it is about 60% done but I think it'll be ready for alpha release in a few days, this should also give enough time for anyone else to give their input)

For the average user, I was thinking something like that but generating the UI widgets dynamically based on their ffmpeg capabilities. And then the advanced option can be like ffmpeg recorder.

Ah, for the avarage user, presets is much more favourable. Most of our users are painters foremost, and a good chunk of that is teenagers who for some reason are doing their school work with Krita?

I'm not sure if dynamically generating sliders will really make sense, as someone who wants to customize a given preset would be better off searching the internet for FFMPEG commands because there's far more documentation and community (like, stackoverflow, blogs demonstrating features, etc) for that. So I think we should just focus on getting really nice presets and making sure it's really easy to go from a preset to a customizable line.

Related: https://bugs.kde.org/show_bug.cgi?id=435610 , not super much though, just something to think about.

emmetoneill closed this task as Resolved.Sep 22 2021, 10:00 PM

Doing some housekeeping and closing this one for now, as we've got things a bit more unified now.

Any other changes to how we handle this or the configuration of FFMPEG versions within Krita can be handled as regular merge requests.

Thanks all!