Re: [5/5] gdiplus: Implement GdipImageSelectActiveFrame.
+ hr = IStream_Clone(image->stream, &stream); + if (FAILED(hr))
That's odd. When does that happen?
+ if (stat == Ok) + { + memcpy(&new_image->format, &codec->info.FormatID, sizeof(GUID)); + free_image_data(image); + if (image->type == ImageTypeBitmap) + *(GpBitmap *)image = *(GpBitmap *)new_image; + else if (image->type == ImageTypeMetafile) + *(GpMetafile *)image = *(GpMetafile *)new_image; + return Ok; + }
Why did you re-implement move_bitmap() ?
Vincent Povirk <madewokherd(a)gmail.com> wrote:
+ hr = IStream_Clone(image->stream, &stream); + if (FAILED(hr))
That's odd. When does that happen?
IStream returned by SHCreateStreamOnFile (wrapped by GdipCreateStreamOnFile) intentionally doesn't implement Clone, there are even tests for that in shlwapi.
+ if (stat == Ok) + { + memcpy(&new_image->format, &codec->info.FormatID, sizeof(GUID)); + free_image_data(image); + if (image->type == ImageTypeBitmap) + *(GpBitmap *)image = *(GpBitmap *)new_image; + else if (image->type == ImageTypeMetafile) + *(GpMetafile *)image = *(GpMetafile *)new_image; + return Ok; + }
Why did you re-implement move_bitmap() ?
An image can be not a bitmap. -- Dmitry.
+ if (stat == Ok) + { + memcpy(&new_image->format, &codec->info.FormatID, sizeof(GUID)); + free_image_data(image); + if (image->type == ImageTypeBitmap) + *(GpBitmap *)image = *(GpBitmap *)new_image; + else if (image->type == ImageTypeMetafile) + *(GpMetafile *)image = *(GpMetafile *)new_image; + return Ok; + }
Why did you re-implement move_bitmap() ?
An image can be not a bitmap.
Yes, but a non-bitmap (metafile) cannot have multiple frames.
Vincent Povirk <madewokherd(a)gmail.com> wrote:
Why did you re-implement move_bitmap() ?
An image can be not a bitmap.
Yes, but a non-bitmap (metafile) cannot have multiple frames.
It actually can have records which represent embedded EMFs. -- Dmitry.
Yes, but a non-bitmap (metafile) cannot have multiple frames.
It actually can have records which represent embedded EMFs.
Really? Well, OK then.
+ if (stat == Ok) + { + memcpy(&new_image->format, &codec->info.FormatID, sizeof(GUID)); + free_image_data(image); + if (image->type == ImageTypeBitmap) + *(GpBitmap *)image = *(GpBitmap *)new_image; + else if (image->type == ImageTypeMetafile) + *(GpMetafile *)image = *(GpMetafile *)new_image; + return Ok; + }
You appear to be leaking new_image.
Vincent Povirk <madewokherd(a)gmail.com> wrote:
+ if (stat == Ok) + { + memcpy(&new_image->format, &codec->info.FormatID, sizeof(GUID)); + free_image_data(image); + if (image->type == ImageTypeBitmap) + *(GpBitmap *)image = *(GpBitmap *)new_image; + else if (image->type == ImageTypeMetafile) + *(GpMetafile *)image = *(GpMetafile *)new_image; + return Ok; + }
You appear to be leaking new_image.
Thanks. I hope that won't prevent accepting other patches in the series. -- Dmitry.
participants (2)
-
Dmitry Timoshkov -
Vincent Povirk