[PATCH v2 1/5] gdiplus: Refactor to split up encode_image_wic().

[Florian Will]

New functions initialize_encoder_wic(), encode_frame_wic() and
terminate_encoder_wic() are useful for implementing GdipSaveAdd() and
GdipSaveAddImage() later.

The WIC encoder is now stored in the new GpImage "encoder" field instead
of a local variable. This makes it possible to keep the encoder active
between multiple gdiplus API calls, which is also useful for
GdipSaveAdd() and GdipSaveAddImage().

Signed-off-by: Florian Will <florian.will at gmail.com>
---
v2: Added an encoder field to GpImage and changed the initialize encoder
function to accept GpImage* instead of IWICBitmapEncoder*. The encoder
is then stored in the image->encoder field. The terminate function now
also accepts GpImage* instead of IWICBitmapEncoder*. It checks if
image->encoder is set, commits and releases it, then sets it to NULL.
That way, it should be more robust against double frees and memleaks.
---
 dlls/gdiplus/gdiplus_private.h |  2 +
 dlls/gdiplus/image.c           | 93 ++++++++++++++++++++++++++--------
 2 files changed, 73 insertions(+), 22 deletions(-)

diff --git a/dlls/gdiplus/gdiplus_private.h b/dlls/gdiplus/gdiplus_private.h
index 8c4fcceded..39caf1a91e 100644
--- a/dlls/gdiplus/gdiplus_private.h
+++ b/dlls/gdiplus/gdiplus_private.h
@@ -72,6 +72,7 @@ extern GpStatus gdip_transform_points(GpGraphics *graphics, GpCoordinateSpace ds
 
 extern GpStatus graphics_from_image(GpImage *image, GpGraphics **graphics) DECLSPEC_HIDDEN;
 extern GpStatus encode_image_png(GpImage *image, IStream* stream, GDIPCONST EncoderParameters* params) DECLSPEC_HIDDEN;
+extern GpStatus terminate_encoder_wic(GpImage *image) DECLSPEC_HIDDEN;
 
 extern GpStatus METAFILE_GetGraphicsContext(GpMetafile* metafile, GpGraphics **result) DECLSPEC_HIDDEN;
 extern GpStatus METAFILE_GetDC(GpMetafile* metafile, HDC *hdc) DECLSPEC_HIDDEN;
@@ -346,6 +347,7 @@ struct GpAdjustableArrowCap{
 
 struct GpImage{
     IWICBitmapDecoder *decoder;
+    IWICBitmapEncoder *encoder;
     ImageType type;
     GUID format;
     UINT flags;
diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c
index dcdc49c6d6..6fee7c3854 100644
--- a/dlls/gdiplus/image.c
+++ b/dlls/gdiplus/image.c
@@ -1827,6 +1827,7 @@ GpStatus WINGDIPAPI GdipCreateBitmapFromScan0(INT width, INT height, INT stride,
     (*bitmap)->height = height;
     (*bitmap)->format = format;
     (*bitmap)->image.decoder = NULL;
+    (*bitmap)->image.encoder = NULL;
     (*bitmap)->hbitmap = hbitmap;
     (*bitmap)->hdc = NULL;
     (*bitmap)->bits = bits;
@@ -2046,6 +2047,8 @@ static void move_bitmap(GpBitmap *dst, GpBitmap *src, BOOL clobber_palette)
     if (dst->image.decoder)
         IWICBitmapDecoder_Release(dst->image.decoder);
     dst->image.decoder = src->image.decoder;
+    terminate_encoder_wic(&dst->image); /* terminate active encoder before overwriting with src */
+    dst->image.encoder = src->image.encoder;
     dst->image.frame_count = src->image.frame_count;
     dst->image.current_frame = src->image.current_frame;
     dst->image.format = src->image.format;
@@ -2078,6 +2081,7 @@ static GpStatus free_image_data(GpImage *image)
     }
     if (image->decoder)
         IWICBitmapDecoder_Release(image->decoder);
+    terminate_encoder_wic(image);
     heap_free(image->palette);
 
     return Ok;
@@ -3744,6 +3748,8 @@ static GpStatus select_frame_wic(GpImage *image, UINT active_frame)
 
     new_image->busy = image->busy;
     memcpy(&new_image->format, &image->format, sizeof(GUID));
+    new_image->encoder = image->encoder;
+    image->encoder = NULL;
     free_image_data(image);
     if (image->type == ImageTypeBitmap)
         *(GpBitmap *)image = *(GpBitmap *)new_image;
@@ -4435,13 +4441,52 @@ GpStatus WINGDIPAPI GdipSaveImageToFile(GpImage *image, GDIPCONST WCHAR* filenam
  *   These functions encode an image in different image file formats.
  */
 
-static GpStatus encode_image_wic(GpImage *image, IStream* stream,
-    REFGUID container, GDIPCONST EncoderParameters* params)
+static GpStatus initialize_encoder_wic(IStream *stream, REFGUID container, GpImage *image)
+{
+    IWICImagingFactory *factory;
+    HRESULT hr;
+
+    TRACE("%p,%s\n", stream, wine_dbgstr_guid(container));
+
+    terminate_encoder_wic(image); /* terminate previous encoder if it exists */
+
+    hr = WICCreateImagingFactory_Proxy(WINCODEC_SDK_VERSION, &factory);
+    if (FAILED(hr)) return hresult_to_status(hr);
+    hr = IWICImagingFactory_CreateEncoder(factory, container, NULL, &image->encoder);
+    IWICImagingFactory_Release(factory);
+    if (FAILED(hr))
+    {
+        image->encoder = NULL;
+        return hresult_to_status(hr);
+    }
+
+    hr = IWICBitmapEncoder_Initialize(image->encoder, stream, WICBitmapEncoderNoCache);
+    if (FAILED(hr))
+    {
+        IWICBitmapEncoder_Release(image->encoder);
+        image->encoder = NULL;
+        return hresult_to_status(hr);
+    }
+    return Ok;
+}
+
+GpStatus terminate_encoder_wic(GpImage *image)
+{
+    if (!image->encoder)
+        return Ok;
+    else
+    {
+        HRESULT hr = IWICBitmapEncoder_Commit(image->encoder);
+        IWICBitmapEncoder_Release(image->encoder);
+        image->encoder = NULL;
+        return hresult_to_status(hr);
+    }
+}
+
+static GpStatus encode_frame_wic(IWICBitmapEncoder *encoder, GpImage *image)
 {
     GpStatus stat;
     GpBitmap *bitmap;
-    IWICImagingFactory *factory;
-    IWICBitmapEncoder *encoder;
     IWICBitmapFrameEncode *frameencode;
     IPropertyBag2 *encoderoptions;
     HRESULT hr;
@@ -4466,20 +4511,7 @@ static GpStatus encode_image_wic(GpImage *image, IStream* stream,
     rc.Width = width;
     rc.Height = height;
 
-    hr = WICCreateImagingFactory_Proxy(WINCODEC_SDK_VERSION, &factory);
-    if (FAILED(hr))
-        return hresult_to_status(hr);
-    hr = IWICImagingFactory_CreateEncoder(factory, container, NULL, &encoder);
-    IWICImagingFactory_Release(factory);
-    if (FAILED(hr))
-        return hresult_to_status(hr);
-
-    hr = IWICBitmapEncoder_Initialize(encoder, stream, WICBitmapEncoderNoCache);
-
-    if (SUCCEEDED(hr))
-    {
-        hr = IWICBitmapEncoder_CreateNewFrame(encoder, &frameencode, &encoderoptions);
-    }
+    hr = IWICBitmapEncoder_CreateNewFrame(encoder, &frameencode, &encoderoptions);
 
     if (SUCCEEDED(hr)) /* created frame */
     {
@@ -4563,13 +4595,30 @@ static GpStatus encode_image_wic(GpImage *image, IStream* stream,
         IPropertyBag2_Release(encoderoptions);
     }
 
-    if (SUCCEEDED(hr))
-        hr = IWICBitmapEncoder_Commit(encoder);
-
-    IWICBitmapEncoder_Release(encoder);
     return hresult_to_status(hr);
 }
 
+static GpStatus encode_image_wic(GpImage *image, IStream *stream,
+    REFGUID container, GDIPCONST EncoderParameters *params)
+{
+    GpStatus status, terminate_status;
+
+    if (image->type != ImageTypeBitmap)
+        return GenericError;
+
+    status = initialize_encoder_wic(stream, container, image);
+
+    if (status == Ok)
+        status = encode_frame_wic(image->encoder, image);
+
+    /* always try to terminate, but if something already failed earlier, keep the old status. */
+    terminate_status = terminate_encoder_wic(image);
+    if (status == Ok)
+        status = terminate_status;
+
+    return status;
+}
+
 static GpStatus encode_image_BMP(GpImage *image, IStream* stream,
     GDIPCONST EncoderParameters* params)
 {
-- 
2.20.1