-
-
Notifications
You must be signed in to change notification settings - Fork 307
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
Conversation
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. |
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? |
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:
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. |
0662ba9
to
85a64f7
Compare
Defaults to true, to match libaom default behaviour libvips/libvips#2983 Co-authored-by: Lovell Fuller <[email protected]>
85a64f7
to
88f0426
Compare
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"; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
I just realized that this explicit option is actually not needed. We have the feature that we can pass arbitrary options to aom like In this case, I'd still keep the option because it is easier to use when it is listed in the |
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