fix(mobile): prevent video freezing/restarting when toggling favorite

[irisfield]

Description

Addresses and fixes #25981. The nature of the problem changed after pulling today's latest changes from main. The root cause is the same though.

Essentially, the video player experienced a lifecycle break whenever an asset's metadata (such as the isFavorite status) was toggled during playback.

Previous behavior (before today): The UI would completely freeze due to a MissingPluginException (caused by attempting to use a disposed controller).

Current behavior (after today's changes from main): The video player silently disposes and recreates itself, causing the video to restart from the beginning.

The root case of the problem:
The video subtree was using ValueKey(asset) as its key. Since the asset object's equality includes its metadata properties, toggling a favorite flag results in a new ValueKey, which to Flutter is a different widget entirely, leading to the destruction and recreation of the NativeVideoPlayerView.

How Has This Been Tested?

I tested the changes on my Pixel 9 Pro.

Screenshots (if appropriate)

Freezing:

prefix-freeze-screen-com-20260221-202730-1771730830702.mp4

Restarting:

prefix-restart-screen-com-20260222-164512-1771803890484.mp4

After Fix:

postfix-screen-com-20260221-202238-1771730538178.mp4

Checklist:

• I have carefully read CONTRIBUTING.md
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation if applicable
• I have no unrelated changes in the PR.
• I have confirmed that any new dependencies are strictly necessary.
• I have written tests for new code (if applicable)
• I have followed naming conventions/patterns in the surrounding code
• All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
• All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

Please describe to which degree, if any, an LLM was used in creating this pull request.

I used it to edit the description for the PR. The one above.

[shenlong-tanwen]

This was updated to use the asset in #22036 (comment)

[goalie2002]

I tested out this PR and it does require this key to change. I knew changing it in #22036 was more of a band-aid fix, but I didn't expect it to be an issue so soon 😅. I'm looking into properly retaining the zoom/pan or finding a better way to completely reset the pan

[irisfield]

Thank you for the review! I am still getting familiar with the code base, but I see now why ValueKey(displayAsset) was used in 58e5caa to reset pan when switching still/motion. Upon testing it, this PR, as-is, would reintroduce the issue described in #22036 (comment).

Using the full asset as the key forces a full teardown when any property changes (e.g. isFavorite), which is what causes the video player to restart and the freeze/MissingPluginException. I can think of two ways to handle both issues.

1. Using a compound key, i.e. ValueKey('${displayAsset.heroTag}_$isPlayingMotionVideo'), so the key changes on still/motion switch but not on favorite

2. Controller reset instead of key-driven rebuild. The idea is basically to keep the stable key (ValueKey(displayAsset.heroTag) and provide a separatePhotoViewController for the video branch and call controller.reset() when the user switches from still to motion (i.e. isPlayingMotionVideo becomes true). That resets pan/zoom at the right time without destroying the widget.

Although 1 is the most minimal, I think 2 is the better solution because it will help avoid other issues like this one down the line. In either case, we probably want to avoid coupling the key to the entire asset object. I would love to handle the implementation. What do you think/prefer?

TLDR: reset state explicitly instead of using key changes to force teardown.

[goalie2002]

Yeah 1 is the quick fix i had in mind too (and was about to PR too). I think seen as I'm looking into retaining the scale, which is the most desirable behaviour anyways, IMO it's fine to just go with the quicker solution. But I'll let the dev team decide that

Edit: I was actually gonna just do ${displayAsset.heroTag}_video and ${displayAsset.heroTag}_photo respectively, but I think the outcome is the same

[irisfield]

Sounds good! I will wait to hear from the dev team as well. Thank you for the quick reply :D

[shenlong-tanwen]

Let's go with 1 in this PR since the change already touches the same line. But would love to see a PR with a more proper solution

[irisfield]

@goalie2002 I hope you do not mind if I take it up. As per @shenlong-tanwen 's recommendation, we will go with solution 1.

I did another self-review of my code and made some minor optimizations. This is my thought process for the changes:

• ValueKey((asset.heroTag, '_video_layer')) instead of ValueKey('${asset.heroTag}_video'), etc - using a Dart record instead of string interpolation to avoid the risk of accidental string collision and for the small performance benefits. I tried to name the keys based on the role of the widget, instead of its type.

I think we can also safely remove the inner keys from child: PhotoView.customChild(...) (ln:437) and child: NativeVideoPlayerView(...) (ln:445) in video_viewer.widget.dart because those branches are already anchored by stable outer key of the Visibility element, meaning that the inner keys are not doing anything. I kept them since the original code had them.

@goalie2002 Incidentally, I am open to continue the discussion to open another PR to come up with a robust solution that addresses both issues. In case that is something you would like, I am sukecchi on Discord.

[uhthomas]

Fixed by #26553 - really appreciate your effort here. It just made sense to combine a few related fixes into one PR.