Microsoft-ui-xaml: AnimatedVisualPlayer::PlayAsync should internally wait for the AnimatedVisual to load instead of silently failing

[jaredhms]

If PlayAsync is called before the AnimatedVisual is loaded, nothing happens, yet the IAsyncAction reports success.

The app developer has to write code like this, whereas it would make more sense for AnimatedVisualPlayer to handle this complexity internally.

`

        // We have to wait until the player successfully loads its animation before we can call Play!
        auto notifyPropertyChanged = m_animatedVisualSource.as<winrt::Windows::UI::Xaml::Data::INotifyPropertyChanged>();
        m_animatedVisualPropertyChangedRegistration = notifyPropertyChanged.PropertyChanged(
            [player, loop = m_loop](const auto& sender, const winrt::Windows::UI::Xaml::Data::PropertyChangedEventArgs& args)
        {
            // If the visual is loaded, we're ready to PlayAsync!
            if (winrt::to_string(args.PropertyName()) == "IsImageLoadingCompleted")
            {
                player.PlayAsync(0, 1.0, loop);
            }
        });

        // Kick off play now if the visuals are already loaded
        if (player.IsAnimatedVisualLoaded())
        {
            player.PlayAsync(0, 1.0, m_loop);
        }

`

An interesting additional bug is that calling PlayAsync before a visual is loaded also breaks AutoPlay (nothing plays after the visual finally loads). Not sure if that should be filed as a separate issue.

[simeoncran]

This is by design. This mimics the behavior of most media (e.g. sound and movie) players.

A PlayOnNextLoadAsync could be added to give the behavior you want, however we figured that most scenarios that use asynchronously loaded content need finer control over what happens if the asynchronous loading doesn't complete on time or as expected. I.e. it's usually simpler to wait for loading, then play, then it is to have to cancel playing if the loading doesn't complete.

BTW, there is another way to react to the the AnimatedVisualPlayer getting content loaded, and that is to listen to the AnimatedVisualPlayer.IsAnimatedVisualLoaded dependency property. That is a more general way of handling async loading as it works for any asynchronous content, not just image loading.

[jaredhms]

There are enough knobs/toggles here to allow for more advanced usage if/when needed, but I don't understand why set source+play isn't the 'golden path' for this API, since that allows you to pick up the API and use it with little to no friction.

More advanced developers can worry more about timing, but I'd expect that most developers won't need to worry about it. Our use case is tied into React Native: render a view, play animations, stop/cancel animations when destroying the view or transitioning state. Requiring us to wire up dependency property changes to know when it's 'safe' to call Play is unnecessary burden.

[chingucoding]

I'm confused now. So is the behavior intentional? Should PlayAsync not wait for the animation? Or should we change that, so that calling PlayAsync waits for the animation to have finished loading, and then start playing?

If the latter is the case, I would be happy to try to fix this. :)

[ranjeshj]

@MikeHillberg @simeoncran
Some options to reduce the code required for this common scenario.

• We could add another parameter to PlayAsync to optionally delay until animation has finished loading, but that would be a breaking change.
• We could add a new overload like PlayOnNextLoadAsync
• We could add a boolean DelayAsyncPlayUntilLoad which modifies the behavior of the current PlayAsync method.

[chingucoding]

Those are all great options. Probably adding an optional parameter is the easiest/most convenient way for developers and also keeps the API surface clean. Should we create a proposal for this?

[MikeHillberg]

I'd expect it to just work too, and for the more advanced case you could monitor events (is there an event)?

What do BitmapImage.Play (aka AniGif) and MediaPlayerElement (MediaPlayer.Play) do?

[chingucoding]

Calling Play on the MediaPlayerElement before it has loaded successfully plays the animation.

The big problem however with that comparison is that all of those methods are not async but run sequentially, so setting source might only return upon successful loading.

[chingucoding]

@MikeHillberg @ranjeshj @StephenLPeters What are your thoughts on this? Should we change the API surface or should PlayAsync change here?

[StephenLPeters]

@simeoncran as well. Personally I'd expect that if I call PlayAsync the animation should play as soon as possible. If that means waiting for the load then so be it. If you need fine control over the timing then you can still respond to the events however you want right?

[simeoncran]

TLDR: Requiring that IsPlaying implies IsAnimatedVisualLoaded is a simple contract that keeps the AVP easier to reason about. We should not optimize for a particular policy for handling delays during asynchronous loading at the expense of simplicity of the contracts.

History: during prototyping we tried various ways of making asynchronous loading easier to deal with. Everything we tried made things worse in some regard and made the code a lot more complex. We ended up deciding to keep it simple and wait to see if one async scenario dominated, and then we could create a wrapper for AVP that made that scenario easier.

I'll try to explain the issues...

PlayAsync plays the currently loaded animation, or it completes immediately if there is no currently loaded animation. PlayAsync guarantees that if it hasn't completed then there is an animation playing. This is a simple contract to explain, and is what you need if you are going to play an animation in response to something (e.g. clicking on a button). If you want to be responsive, you do not want PlayAsync to wait for loading that could take a long time. This is the golden path for most people using codegenned Lotties.

If you have an IAnimatedVisualSource that loads asynchronously you will want some policy to deal with the loading taking a long time. That case is not so golden - you have to write more code, but that's because we don't assume what your policy should be. I do not believe that waiting is the right policy in most cases, but what would I know... I have no idea what the important cases for asynchronous loading are. Do we have data?

Consider what would happen if we assumed that the policy should be that you call PlayAsync and nothing happens until loading completes after some arbitrary delay. That will work well in cases where you don't care about how responsive the animation is, or you are certain that the loading will always complete within the bounds of what you consider responsive. But if doesn't help if you need to implement your own policy for slow loading - in that case you'll have to be aware of when the loading completes, and you'll have to cancel the PlayAsync if loading doesn't complete soon enough. It also adds some complexity to the contracts for other methods on AVP:
Pause()
Resume()
These methods refer to the current play, or else they do nothing. This is easy because IsPlaying implies IsAnimatedVisualLoaded. We would need to keep track of the Pause and Resume states and if loading completes while the play is in the pause state, pause the animation as soon as it's loaded. Currently we don't have to worry about this because an unfinished PlayAsync task implies that there is an animation to pause or resume.
SetProgress(...)
This refers to the currently loaded AnimatedVisual and does nothing if called while there is no AnimatedVisual loaded. If PlayAsync(...) was to wait for loading then we'd have a state where there is a current play but no AnimatedVisual loaded. In that state, SetProgress(..) would be equivalent to Stop() (i.e. it would stop the play, but the progress would not be set). Or maybe we would have SetProgress(...) set the progress on the next successful load. In that case SetProgress would not complete the PlayAsync if IsAnimatedVisualLoaded is false, but then PlayAsync would complete as soon as IAnimatedVisualLoaded is true.

Sure, we can do all this, but it makes things more complicated. And this is all to simplify the code for a very rare case (I say that based on how common are the Lottie files that I see on LottieFiles.com that require image loading (image loading is the case that started this discussion)).

[jaredhms]

"_That will work well in cases where you don't care about how responsive the animation is, or you are certain that the loading will always complete within the bounds of what you consider responsive._ "

Agreed, and this is true for all of the animations in our app. If they don't run by the time we need them to (loading screen, etc.), we just cancel them and move on.

We have two choices for the 'golden path' of this API (set source + play()): either default to waiting for the visual to load, or don't wait, and leave that up to the developer to worry about.

The latter option makes this API _much_ more difficult to pick up and use, so I don't agree that it should be the default choice. Why make the more complicated path be the default? Timing isn't important in our animations, but now we _have to_ worry about timing just to use this API.

Forcing all developers to wire up a dependency property notification even if they don't care about timing makes everyone's lives more difficult vs. scoping the complexity to those who need to worry about timing anyway.

[simeoncran]

We're not forcing all developers to do this extra step, only those cases where IAnimatedVisualSource is asynchronous and responsiveness isn't important. That's exactly your scenario, but it's not the common case for Lottie.

BTW, the workaround shown in your example code is not what I would recommend. It will only work for some types of generated code. I would expect that you could write a wrapper WaitForLoadThenPlayAsync that takes just an AnimatedVisualPlayer and causes the Task to not complete until the next load and the play completes. It would avoid the problems that I discussed because it doesn't need to solve the problems with the state contracts and method behaviors. You could think of it as a way of choosing the policy you want for PlayAsync without requiring us to complicate AnimatedVisualPlayer.

Let's at least provide example code for that function.

[simeoncran]

To make AVP start playing an asynchronously loaded animated visual as soon as it finishes loading:

    static void WaitForLoadThenPlayAsync(AnimatedVisualPlayer player)
    {
        if (player.IsAnimatedVisualLoaded)
        {
            _ = player.PlayAsync(0, 1, true);
        }
        else
        {
            player.RegisterPropertyChangedCallback(AnimatedVisualPlayer.IsAnimatedVisualLoadedProperty, (sender, dp) =>
            {
                _ = player.PlayAsync(0, 1, true);
            });
        }
    }

This is similar to what was shown at the start of this issue, but it doesn't require referencing a particular animated visual source so you can just write it once. But this is overkill for your case because you don't care about responsiveness and you expect your images to load quickly, so...

_Turn off asynchronous loading_. This will allow you to use AVP as if you had a synchronously loaded animated visual, with behavior that should be the same as what you'd get from the web player, i.e. the Lottie will start playing immediately and the images will pop in as they load. Typically the images will appear almost immediately and the user will not be aware of the asynchrony.

Lottiegen enables asynchronous loading of animated visuals that use images, but it generates a property so you can turn off asynchronous loading: IsAnimatedVisualSourceDynamic. For example, to auto-play a Lottie that has images without using any code behind:

    <muxc:AnimatedVisualPlayer x:Name="Player">
        <animatedVisuals:Splash_Intro_Large IsAnimatedVisualSourceDynamic="False" />
    </muxc:AnimatedVisualPlayer>

I suspect we should have made IsAnimatedVisualSourceDynamic default to false so as to avoid this issue bothering anyone. Tracking that here: https://github.com/windows-toolkit/Lottie-Windows/issues/340

[chingucoding]

@simeoncran So would you advice against making PlayAsync wait for the AnimatedVisual to load given that in some cases this might make the animation very unresponsible? Or should we wait for the visual to load when PlayAsync is being called?

[simeoncran]

@chingucoding right, I would advise against changing the design to make this rare case easier at the expense of making all cases harder to reason about. Is this actually causing anyone any pain that can't be solved by simply setting IsAnimatedVisualSourceDynamic to false?

[chingucoding]

I would argue that it's more in the range of "nice to have" rather than a major pain point, though I haven't worked that much with AVP. I can see why it would make sense for very simple scenarios to have PlayAsync to wait, however as you noted, more complex scenarios will get harder to reason about. I'm inclined to say that it might be better to leave as is then, @StephenLPeters @jaredhms, your thoughts?

[simeoncran]

What is the simple scenario where you'd prefer to have it wait rather than making the content non-asynchronous to begin with?

[chingucoding]

Thinking about it, when you can't make it non-asynchronous (e.g. loading files or something like that), you need to handle failure anyway. I can't think of a simple scenario where making the content non-asynchronous wouldn't work. Now I am even more convinced that the current is probably the best way to handle this, thank you @simeoncran for helping us/me this scenario better!