fix(mobile): #15182 Video memories no longer play #15210
Conversation
| if (currentAssetPage.value == 0) { | ||
| Future.delayed(const Duration(milliseconds: 1)).then((_) { | ||
| final asset = currentMemory.value.assets[0]; | ||
| ref.read(currentAssetProvider.notifier).set(asset); |
There was a problem hiding this comment.
The current asset should be set before this page is being built. Setting it in a future on the page itself is hacky.
There was a problem hiding this comment.
I agree, but where would you recommend? This is really the earliest place it can be done that makes sense since we don't know the asset prior to this point (I believe - I'm still getting used to the code-base and Flutter).
I tried doing it in a useEffect, but Flutter doesn't like that. I used a future because we do the same already for caching the "next" asset (and Flutter actually suggested I use a Future when I tried it in useEffect).
There was a problem hiding this comment.
You can set it before pushing the route here
There was a problem hiding this comment.
Not sure I like setting it before the asset is current in a tap action, but 🤷
Done.
There was a problem hiding this comment.
It's what the Riverpod dev recommends; it avoids rebuilding the widget one frame after it's built and means the widget can assume the current asset is set without having to bootstrap itself.
It depends on how long the animation takes. The video initialization kind of competes with Flutter animations, so the timers are to make sure the video waits for the animation. The memory animation might be slow enough that the existing timers aren't enough. Other animations like swiping and maybe the hero animation were made faster so the video can wait for them without waiting too long. |
I believe this was introduced along with the introduction of the currentAssetRepository.
The fix (for #15182) is relatively simple by just ensuring the right currentAsset gets set on memoryPage load and transition.
Unfortunately, while this fixes the issue, I now have pretty weird stutter that shows for a few hundred ms between the preview image and playing the actual video on my Pixel 8. There is already some timers to try to prevent "stutters" (which may be related), but I found I had to increase them significantly (300ms to ~1s) to hide the issue completely. I'm guessing it's related to funniness in Android's video surface (which is known to be a bit dodgy); but I tried messing around with waiting till the video was initialised/loading/playing, but couldn't get it going.
Since I'm not sure if the above stuttering is a regression (I'm very new to using the app) - I thought I'd raise this PR but ask someone who's more familiar with this area of logic to have a look.