search

master-of-zen / Av1an

Cross-platform command-line AV1 / VP9 / HEVC / H264 encoding framework with per scene quality encoding
GNU General Public License v3.0
1.4k stars 147 forks source link

Improve setting default encode parameters #798

Open FlyingWombat opened 5 months ago

FlyingWombat commented 5 months ago

This improves upon setting default encoding parameters. The gist of this pull request is improving the tile and worker estimation, and keeping those values when setting video-params. My main use-case for this is that I sometimes have batches of mixed resolution videos (e.g. 1080p and 4k) that I want to encode with the same settings -- and not have to go through one by one to specify tile count.

Some of the commits are loosely related, and I wasn't sure if I should make separate pull requests for them, so they are included here. I am a beginner with rust, so please correct me if the formatting is bad, or if there is a better way to implement the changes.

shssoichiro commented 5 months ago

Thanks, I need to look through more in detail but a lot of this looks like an improvement.

FlyingWombat commented 5 months ago

The default tiling estimation was already there, this pull request doesn't change that -- just makes it actually useful. Av1an already, by default will determine tiles and set encoder parameters accordingly. But as is, if --video-params is specified, all default parameters set by Av1an are ignored, such as the (useful) tile estimation. What this pull request fixes is that now the tiling estimation can be actually used with user-specified encoder parameters. The "settings: merge video_params..." commit is what does this, and also has the --force flag ignore Av1an's default encoder parameters altogether. Parameters in --video-params will override, so if you want multiple keyframes per chunk, just set --keyint in --video-params. If we don't want Av1an to set encoder parameters automatically, then we should remove the tiling estimation and default parameters code altogether.

Considering tiles in worker estimation is better because (except for svt-av1) tiling changes the CPU resources used by each worker. Not considering tiles in worker estimation would over-commit the CPU and use more RAM than necessary.

Av1an already prints the encoder parameters being used, and I added a message for that in the log file in this pull request.

FlyingWombat commented 5 months ago

You can still have the old behavior, where Av1an doesn't set default parameters at all. This pull request has the --force flag ignore all of Av1an's default encoder parameters. Do you think I should put this option in a separate flag, in case a user wants to ignore default arguments, but still have the syntax check?

We could add a flag to opt-out of just tile estimation e.g. --no-tile, if you think a significant portion of people would use it.

The main reason I made the change to merge video_params into defaults, is because I wanted to use the tiling and worker estimation. Tiling and worker estimation, and all the default parameter code for that matter, is unused when --video-params is set -- which pretty much everyone does to at least set --crf. There has been confusion about this behavior before, see #662 . This pull request fixes that. Without default parameter merging, I'd have to clutter my encoding parameters with e.g. --keyint 0 --scd 0 --rc 0, which only need to be set because I'm using av1an, and I'd have to make an external wrapper for av1an to estimate tiles based on resolution.

We may need to change the numbers used for tile estimation. I also thought it was too many tiles, and changed it in my local copy of Av1an.

FlyingWombat commented 5 months ago

@damian101 Do you have any specific changes to propose for this pull request? I'm not sure what to do with your feedback.

Does the added behavior for the --force flag meet your needs? Or, would adding a new flag specifically to control tile estimation be better? (but then we're adding another cli flag) E.g. --no-tile to disable tile estimation; or --auto-tile to enable tile estimation. Should tile estimation be enabled by default, or disabled by default?

The encoding parameters are printed to the screen and the log file.

FlyingWombat commented 4 months ago

@damian101 Per your request, I've added a "--tile-auto" flag. Av1an will now only do tile estimation (and automatic setting of tiling encode params) if this flag is present. It took me a while to get back to this, but I think now all the discussion points have been addressed.

log2 conversion is done in encoder.rs::get_default_arguments for applicable encoders.