This should be updated to reflect the actual set of extensions in the PR.

Applying this to all gif files seems like a large hammer; not all gifs are animated. Shouldn't we be examining the file's header to determine whether it's an animated gif?

Hm, I'll look into implementing this; however I think the header doesn't contain info about animation, the file needs to be scanned to look for "local image descriptors" (> 1)

There's a promising "Movie" class that might be useful to see if an image is animated or not. I'll use it to check both GIF and PNG (potentially animated), and leave the extensions for APNG (definitely animated) and SVG (vector).

Adding to what Stuart has commented:

For non-animated gif, we should scale it as it works as is.
For animated gif, we can scale it by scale all the frames of the git then combine them again. See: flutter/flutter#34134 (comment)

We can make this PR to exclude file extensions that cannot be scaled, which doesn't include gifs. And create another PR to scale gif.

@cyanglaz in theory there is more than 1 animated file format available on Android:

Animated GIFs, WebP and HEIF via list of Android formats

IMHO, the MR currently resolves a issue in that the GIF file looses animation, which is the No. 1 purpose of using a GIF file. I understand your point to "do it right", but think this would delay the product iteration by a lot. Why not merging this change with determined behaviour (as breaking change) and consider the next step after community feedback?

IMHO, the MR currently resolves a issue in that the GIF file looses animation, which is the No. 1 purpose of using a GIF file.

And the only reason to pass max width/height is to cause the image to be resized. You are proposing breaking something that currently works in order to fix something that is sometimes currently broken. That is not a clear improvement.

I understand your point to "do it right", but think this would delay the product iteration by a lot.

Doing things correctly sometimes takes longer, yes. Rapidly moving between different broken states isn't something clients of software generally view as a feature.

Why not merging this change with determined behaviour (as breaking change) and consider the next step after community feedback?

If we are going to do an interim fix, it would be something along the lines of my non-breaking suggestion, so that we do not:

• actively regress people
• introduce a breaking change for something that is not necessarily a net improvement, and which would need another breaking change to change again later to improve

Breaking changes are disruptive; we do not want to set ourselves up to make several when there's a perfectly good way not to.

@stuartmorgan OK understood.

This is a breaking change, and would need to be versioned accordingly.

I think a better option would probably be a new, optional boolean to apply resizing only if we're confident that we can do it without destroying information, defaulting to false. That makes this a non-breaking change (and would make things like the animated vs non-animated gif issue something we could punt on).

So, given the current pickImage method signature:

Future<XFile?> pickImage({
    required ImageSource source,
    double? maxWidth,
    double? maxHeight,
    int? imageQuality,
    CameraDevice preferredCameraDevice = CameraDevice.rear,
  }) {
    return platform.getImage(
      source: source,
      maxWidth: maxWidth,
      maxHeight: maxHeight,
      imageQuality: imageQuality,
      preferredCameraDevice: preferredCameraDevice,
    );
  }

The new boolean would be called safeResizeOnly: false and be described as follows:

When [safeResizeOnly] is set to true and a resize would incur a loss of information, the original image will be returned. This applies to animated formats like GIF or APNG, which can not be resized on Android without loosing the animation.

WDYT, @stuartmorgan (naming variables is not easy ;-)).

Side note: When you said:

only if we're confident that we can do it without destroying information

.. I started to wonder what happens to the EXIF metadata on resize. It was on the radar previously https://github.com/flutter/flutter/issues?q=is%3Aissue+exif+image_picker+is%3Aopen so may warrant another check in the future.

The new boolean would be called safeResizeOnly: false and be described as follows:

When [safeResizeOnly] is set to true and a resize would incur a loss of information, the original image will be returned. This applies to animated formats like GIF or APNG, which can not be resized on Android without loosing the animation.

That sounds good, except s/would/could/, and I would explicitly say that it's a heuristic that may be adjusted over time. I think your wording would be misleading about, e.g., the initial planned handling of GIFs (where it will fail to resize them even when not animated).

I started to wonder what happens to the EXIF metadata on resize

You could say something like "fundamentally change the image" instead of "incur a loss of information", to make it clearer that it's not just any loss of information.

That's probably a good idea anyway, because as currently written any resize down would actually be forbidden because it loses information.

Whatever the change ends up being, the doc change will need to be applied to all the methods that take a size.

Adding to what Stuart has commented:

For non-animated gif, we should scale it as it works as is.
For animated gif, we can scale it by scale all the frames of the git then combine them again. See: flutter/flutter#34134 (comment)

We can make this PR to exclude file extensions that cannot be scaled, which doesn't include gifs. And create another PR to scale gif.