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.