Re: [3/3] gdiplus: Add an ability to cache bitmap properties and use distinct metadata loaders for different image formats.

[Vincent Povirk]

> @@ -2123,6 +2125,8 @@ static void move_bitmap(GpBitmap *dst, GpBitmap *src, BOOL clobber_palette)
>      if (dst->metadata_reader)
>          IWICMetadataReader_Release(dst->metadata_reader);
>      dst->metadata_reader = src->metadata_reader;
> - dst->prop_count = src->prop_count;
> - dst->prop_item = src->prop_item;
> if (dst->image.stream)
>     IStream_Release(dst->image.stream);
> dst->image.stream = src->image.stream;

I think you should be freeing dst->prop_item here.
> @@ -2850,6 +2923,12 @@ GpStatus WINGDIPAPI GdipGetAllPropertyItems(GpImage *image, UINT size,
>  if (prop_count != count || prop_size != size) return InvalidParameter;
>
> - if (((GpBitmap *)image)->prop_item)
> - {
> -
>    memcpy(buf, ((GpBitmap *)image)->prop_item, prop_size);
> -
>    return Ok;
> - }
> - reader = ((GpBitmap *)image)->metadata_reader;
>    if (!reader) return PropertyNotFound;

I think you have to change the values here so they point into the user buffer.
> +typedef GpStatus (*metadata_reader_func)(GpBitmap *bitmap, IWICBitmapDecoder *decoder, UINT frame);

...
> -
>        if (metadata_reader)
> -
>            metadata_reader(bitmap, decoder, active_frame);
> -
>        else if (IWICBitmapFrameDecode_QueryInterface(frame, &IID_IWICMetadataBlockReader, (void

Either your metadata reader function should return void, or you should
check the returned status.

Otherwise, this looks good to me.