heif_encoder_aom.cc: Fix input_image leak on error

[wantehchang]

No description provided.

[farindk]

Thank you. I've added a comment to make it clear why the image is (very creatively) released at that very position.

[wantehchang]

Thank you, Dirk.

Another solution is to call aom_img_free() inside encode_frame() before it returns an error.

[farindk]

@wantehchang Yes, but then 'alloc' and 'free' would not be on the same (function call) level. I prefer your solution.

[wantehchang]

Dirk: In that case, we may want to move the aom_codec_destroy() call in encode_frame() to its caller:

diff --git a/libheif/plugins/heif_encoder_aom.cc b/libheif/plugins/heif_encoder_aom.cc
index 5baa8db..7f80560 100644
--- a/libheif/plugins/heif_encoder_aom.cc
+++ b/libheif/plugins/heif_encoder_aom.cc
@@ -741,7 +741,6 @@ static heif_error encode_frame(encoder_struct_aom* encoder, aom_codec_ctx_t* cod
         heif_suberror_Encoder_encoding,
         encoder->set_aom_error(aom_codec_error_detail(codec))
     };
-    aom_codec_destroy(codec);
     return err;
   }
 
@@ -1029,6 +1028,7 @@ struct heif_error aom_encode_image(void* encoder_raw, const struct heif_image* i
   aom_img_free(&input_image);
 
   if (err.code != heif_error_Ok) {
+    aom_codec_destroy(codec);
     return err;
   }
 

[farindk]

Or even better: remove the useless encode_frame() completely, as in my patch above.

[wantehchang]

Dirk: Your patch above brought back the leak of input_image!

Here is one way to fix the leak:

diff --git a/libheif/plugins/heif_encoder_aom.cc b/libheif/plugins/heif_encoder_aom.cc
index 6cac115..692d11d 100644
--- a/libheif/plugins/heif_encoder_aom.cc
+++ b/libheif/plugins/heif_encoder_aom.cc
@@ -1004,6 +1004,11 @@ struct heif_error aom_encode_image(void* encoder_raw, const struct heif_image* i
                          0, // only encoding a single frame
                          1,
                          0); // no flags
+
+  // Note: we are freeing the input image directly after use.
+  // This covers the usual success case and also all error cases that occur below.
+  aom_img_free(&input_image);
+
   if (res != AOM_CODEC_OK) {
     err = {
         heif_error_Encoder_plugin_error,
@@ -1014,15 +1019,6 @@ struct heif_error aom_encode_image(void* encoder_raw, const struct heif_image* i
     return err;
   }
 
-
-  // Note: we are freeing the input image directly after use.
-  // This covers the usual success case and also all error cases that occur below.
-  aom_img_free(&input_image);
-
-  if (err.code != heif_error_Ok) {
-    return err;
-  }
-
   encoder->compressedData.clear();
   const aom_codec_cx_pkt_t* pkt = NULL;
   aom_codec_iter_t iter = NULL; // for extracting the compressed packets

[farindk]

Thanks. I really should not be working on three different projects at the same time ...