Skip to content

Avoid self-copy data loss in avifImageCopy()#3234

Closed
jmestwa-coder wants to merge 1 commit into
AOMediaCodec:mainfrom
jmestwa-coder:avifimagecopy-self-copy
Closed

Avoid self-copy data loss in avifImageCopy()#3234
jmestwa-coder wants to merge 1 commit into
AOMediaCodec:mainfrom
jmestwa-coder:avifimagecopy-self-copy

Conversation

@jmestwa-coder
Copy link
Copy Markdown
Contributor

Summary

avifImageCopy() does not handle self-copy correctly.

When the same image is passed as both source and destination:

avifImageCopy(image, image, AVIF_PLANES_ALL);

the function frees the image planes before copying, causing owned image data to be discarded while still returning AVIF_RESULT_OK.

Fix

Add an early return for self-copy operations:

if (dstImage == srcImage) {
    return AVIF_RESULT_OK;
}

This preserves existing image data and avoids unnecessary work when the source and destination are the same object.

@y-guyon
Copy link
Copy Markdown
Contributor

y-guyon commented Jun 1, 2026

Thank you for your interest in libavif.

I would consider this as incorrect use of the API and in my opinion there is no need to handle that case nor explain it in the API comment.

Why would a user call avifImageCopy(image, image, planes)?

@jmestwa-coder
Copy link
Copy Markdown
Contributor Author

@y-guyon Thank you for the clarification.

I viewed self-copy as a valid operation because the API does not currently document that the source and destination must be different.

My concern was that the function currently returns AVIF_RESULT_OK while modifying the image when dstImage == srcImage.

Would you prefer this to remain undocumented caller responsibility, or would documenting dstImage != srcImage as a precondition be worthwhile?

@wantehchang
Copy link
Copy Markdown
Member

In API design it seems more elegant to allow self copy and handle it properly, similar to the assignment operator = in C++.

But I agree with Yannis that calling avifImageCopy(image, image, planes) is extremely unlikely and would almost always indicate a programming error. We probably should strive to detect such bugs rather than ignore them. I suggest we change your check for self copy to return an error.

There are two other libavif public functions that take two avifImage parameters:

  • avifImageSetViewRect: This should require dstImage != srcImage. See PR Disallow dstImage == srcImage #3236.
  • avifImageComputeGainMap: I think it is fine if baseImage == altImage.

@wantehchang
Copy link
Copy Markdown
Member

This PR has been replaced by PR #3236.

@wantehchang wantehchang closed this Jun 3, 2026
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.

3 participants