[LOCKEDFILE]

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

• Adds a configuration option to set the minimum frame time for GIF images.

  if (_frameDuration! <= const Duration(milliseconds: 10)) {
    _frameDuration = const Duration(milliseconds: 100);
  }

in multi_image_stream_completer.dart

⤵️ What is the current behavior?

• Currently, GIF images do not have a configurable minimum frame time, which can lead to inconsistent or overly rapid playback on some devices.

🆕 What is the new behavior (if this is a feature change)?

• This PR introduces a feature to configure the minimum frame time for GIF images, allowing smoother playback and control over animation speed, especially on platforms that impose minimum frame rate limits.

2024-12-13.5.13.59.mov

💥 Does this PR introduce a breaking change?

No

🐛 Recommendations for testing

Test GIF playback with various minimum frame times to ensure the configuration functions as expected.
Confirm that the feature works across different devices and platforms, including browsers with built-in frame rate limits like Chrome.

📝 Links to relevant issues/docs

• the Chrome browser enforces a minimum GIF frame time of 100ms by default, which limits playback speed for faster animations. (See source)

🤔 Checklist before submitting

• All projects build
• Follows style guide lines (code style guide)
• Relevant documentation was updated
• Rebased onto current develop

[rickdijk]

This repository now uses main as its only branch, so I have retargeted this PR from develop to main. Nothing about the review changes. If GitHub now shows a conflict, a rebase onto main will clear it, and we will follow up on that separately.

[rickdijk]

Thanks for this! Clamping near-zero frame delays to 100ms matches what browsers do, and it would fix GIFs that play too fast here on mobile. A few things before we can merge:

1. The description promises a configurable option, but the clamp is hardcoded and always on. That's a silent behavior change for anyone who uses fast frames on purpose. Could you make it opt-in instead, e.g. a Duration? minFrameDuration on CachedNetworkImageProvider / CachedNetworkImage (default null, so nothing changes) passed to MultiImageStreamCompleter?
2. This applies to every animated format (WebP, APNG), not only GIF. Please mention that in the docs for the new parameter.
3. Please pull 10ms and 100ms out into named constants with a comment pointing at the browser convention, and clamp _nextFrame!.duration before assigning it rather than reassigning _frameDuration.
4. Please add a test in the existing completer tests using a fake codec with 0ms, 10ms and 11ms frames, to pin the boundary.
5. Please rebase onto main, bump the version (minor, since this adds API) and add a dated CHANGELOG entry.

Note that Image.network doesn't clamp, so if you'd like this to be the default everywhere, it's worth raising with flutter/flutter too.

[LOCKEDFILE]

Thanks for the feedback. I’ve updated the PR:

• Added an opt-in Duration? minFrameDuration to CachedNetworkImageProvider and CachedNetworkImage, defaulting to null.
• Documented that the setting applies to animated formats such as WebP and APNG.
• Added named constants for the 10 ms threshold and 100 ms browser minimum, and applied the clamp before assigning the frame duration.
• Added a fake-codec test covering 0 ms, 10 ms, and 11 ms frames.
• Bumped the package version to 4.1.0, added a dated changelog entry, and rebased the branch onto main.

[rickdijk]

Thanks for the quick turnaround. Opt-in with a null default, the docs mentioning WebP and APNG, the named constants, the version bump and the changelog entry all look good. CI hasn't run on this PR yet, so I ran it locally on feb5b72, and three things still need fixing:

1. The new test fails. flutter test reports +51 -1, and the failure is minimum frame duration clamps 0ms and 10ms, not 11ms. Details are inline.

2. dart format --set-exit-if-changed fails on cached_network_image_provider.dart and multi_image_stream_completer.dart, so the CI format job will be red. Run dart format on those two files only (not dart format .).

3. The setting doesn't do what its name says. I measured how long frames are held with this branch:

   minFrameDuration	0, 5 and 10ms frames	11ms frame	50ms frame
   null	unchanged	11	50
   20ms	100	11	50
   100ms	100	11	50
   150ms	150	11	50

   Any value below 100ms behaves exactly like 100ms, and frames longer than 10ms never get raised to the minimum. More on this inline.

flutter analyze is clean. Nothing breaks for existing users: with the default null, frame timing and provider equality are unchanged.

[rickdijk]

This mixes two rules. The clamp only fires for frames of 10ms or less, and when it does, it uses max(minFrameDuration, 100ms). As a result, minFrameDuration: Duration(milliseconds: 20) holds a 0ms frame for 100ms, and minFrameDuration: Duration(milliseconds: 150) leaves a 50ms frame at 50ms. Neither matches the name, and the doc comment's "never less than 100ms" means the value someone passes is quietly overridden.

The 100ms floor is something I asked for in the first review, and combining it with the opt-in setting is what made this confusing. Please pick one rule and make the docs and tests match it. I'd suggest keeping the 10ms browser threshold and dropping the hidden 100ms floor:

final frameDuration = _nextFrame!.duration;
_frameDuration =
    minFrameDuration != null && frameDuration <= _browserMinimumFrameDelay
    ? minFrameDuration
    : frameDuration;

Then document it as "frames with a delay of 10ms or less are shown for minFrameDuration instead; pass 100ms to match browsers". _browserFrameDelayFallback would then only be needed in that doc comment.

Nit: the blank line this change leaves after _shownTimestamp = timestamp; can go.

[rickdijk]

The test fails here first. On current main, the completer emits a clone of the frame image (#1062), so an identity comparison against image20x10 never matches. The frame also needs await tester.idle(); before the pump(), like the later frames have. Something like:

await tester.idle();
await tester.pump();
expect(images, hasLength(1));
expect(images.single.isCloneOf(image20x10), isTrue);

[rickdijk]

The timing is off by one frame. A frame's duration controls how long that frame stays on screen, so the time until frame 4 appears is frame 3's hold (the 10ms frame, clamped to 100ms), not 0. The 0ms clamp is what the frame-3 check measures, the 10ms clamp is measured here, and the 11ms case is measured by the frame-5 check. This should be pump(99ms) → still 3, then pump(1ms) → 4, and the comments should match. With those two fixes the clamping checks pass, but the final check still failed for me, so please run the test end to end once the rule below settles.

Once the rule in the completer settles, it would also be good to cover:

• a minFrameDuration above the threshold, to show the value you pass is the value used
• that CachedNetworkImage and CachedNetworkImageProvider pass the value through, and that two providers with a different minFrameDuration are not == (equality and hashCode changed, and nothing tests it)

[rickdijk]

dart format --set-exit-if-changed rejects this (and the ternary in multi_image_stream_completer.dart). The formatter wants:

  int get hashCode => Object.hash(
    cacheKey ?? url,
    scale,
    maxHeight,
    maxWidth,
    minFrameDuration,
  );

[LOCKEDFILE]

Addressed the frame timing and formatting feedback. minFrameDuration now applies the provided value only to frames of 10ms or less; longer frames retain their codec duration. Updated the timing, provider equality, and widget propagation tests. flutter analyze and flutter test pass.