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

AOM encoder: Expose control over Intra Block Copy (intrabc) #1408

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

lovell
Copy link
Contributor

@lovell lovell commented Nov 22, 2024

The "intra block copy" feature of the AV1 codec can lead to an order of magnitude increase in encoding time when certain conditions/dimensions are met.

This feature, which is switched on by default in libaom but not always triggered, searches for similar areas within the same frame, e.g. repeating text, but at a high CPU cost. My understanding is that it can also prevent the use of (decoding?) loop filters, so useful features like deblocking and deringing are skipped.

There is more context and discussion, including comments from libaom maintainers, at libvips/libvips#2983

/cc @kleisauke who suggested the original patch

@farindk
Copy link
Contributor

farindk commented Nov 22, 2024

Thanks. Did you see any significant reductions in file size when IntraBC mode is used? If it provides some improvements at least for some types of images (e.g. screenshots), it might make sense to add an option to enable it.

@lovell
Copy link
Contributor Author

lovell commented Nov 22, 2024

Screenshots and subtitles are good examples of the sort of thing I imagine this feature was targeting. Some quick testing suggests up to 10% reduction in file size (but with the 10x slow down) for the libaom-default enabled state vs this PR.

I think that's enough to warrant this being an option, so I'll go ahead and make an update to expose it. Should the default libheif behaviour differ from libaom? Is a consistent encoding time for a given "speed" setting more important than file size?

@farindk
Copy link
Contributor

farindk commented Nov 22, 2024

Handling encoding options is difficult because every encoder has its own options and compression vs. bitrate vs. time characteristic. My proposal for the future would be to have options in two level:

  • one low level that maps 1:1 to the options of the underlying encoder, and
  • high level options that are more meaningful for the user. This could include things like "content type", "speed vs quality".

In that sense, it would be better to have the same defaults in the low level options as the encoder library. In the case above, a "disable-blockcopy" would be more consistent.

If you didn't start yet, I can take over and implement this.

@lovell lovell force-pushed the aom-encoder-intrabc-off branch from 0662ba9 to 85a64f7 Compare November 22, 2024 14:44
@lovell lovell changed the title AOM encoder: always disable IntraBC, prevents occasional 10x slowdown AOM encoder: Expose control over Intra Block Copy (intrabc) Nov 22, 2024
Defaults to true, to match libaom default behaviour

libvips/libvips#2983

Co-authored-by: Lovell Fuller <[email protected]>
@lovell lovell force-pushed the aom-encoder-intrabc-off branch from 85a64f7 to 88f0426 Compare November 22, 2024 15:51
@farindk farindk merged commit e04390d into strukturag:master Nov 22, 2024
26 of 35 checks passed
@farindk
Copy link
Contributor

farindk commented Nov 22, 2024

Great. Thanks.

@@ -162,6 +163,7 @@ static const char* kParam_alpha_min_q = "alpha-min-q";
static const char* kParam_alpha_max_q = "alpha-max-q";
static const char* kParam_lossless_alpha = "lossless-alpha";
static const char* kParam_auto_tiles = "auto-tiles";
static const char* kParam_intra_block_copy = "intra-block-copy";
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to name this parameter "enable-intrabc" to match the name used in libaom. On the other hand, I am not sure if libheif has tried to make these parameter names match the names used in libaom, so this may not be worth the trouble.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I believe this makes sense. Parameters that correspond 1:1 to codec parameters should be identical to reduce the confusion and cognitive load.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the comment, Dirk. This will require a corresponding change to libvips (see libvips/libvips#4284) to use the new parameter name. During the transitional period libheif can support both the old and new parameter names.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I've notified them at libvips/libvips#4284.
One can also use heif_encoder_set_parameter_string(..., "aom:enable-intrabc", "0") which sets the parameter directly through aom_codec_set_option() and which works even for past versions of libheif.

@farindk
Copy link
Contributor

farindk commented Nov 27, 2024

I just realized that this explicit option is actually not needed. We have the feature that we can pass arbitrary options to aom like -p aom:enable-intrabc=0, which calls aom_codec_set_option(&codec, "enable-intrabc", "0") with the same effect.

In this case, I'd still keep the option because it is easier to use when it is listed in the heif-enc -P parameters overview.

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.

4 participants