[Bug] MediaElement won't update if <= 300ms from current Position

[albertt425]

MediaElement is hard-coded to ignore small position updates (<= 300ms).

https://github.com/xamarin/XamarinCommunityToolkit/blob/21bec3e669a78ea4c55f25985001cd73f8498db9/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/MediaElement/MediaElement.shared.cs#L99

This works in the Xamarin.Forms experimental MediaElement. This is useful on platforms that support frame by frame Position changes. This code may have been added for performance, so it could be useful to make the check optional or interval configurable (although I'd prefer to simply let the caller manage it).

Steps to Reproduce

Load up a 30fps video in MediaElement. Stop video at any position. Add 50ms to current position.

Expected Behavior

Video updates to new frame.

Actual Behavior

Position update is ignored.

Basic Information

• Version with issue: all 1.0.0
• Last known good version: Xamarin.Forms (using experimental)
• IDE: any
• Platform Target Frameworks: all
• Nuget Packages:
• Affected Devices:

[pictos]

@peterfoot can you share the historical context of this implementation?

[peterfoot]

@pictos I remember there being an issue when events from the native control could cause the code to try and set the new position again causing a jump. I don't know if this still applies now the way the Position events are managed has changed.

[albertt425]

Suggestions on how to proceed? @peterfoot, do you recall the platform that had the problem? I can poke around on UWP or Android, but do not have a Mac.

[peterfoot]

@albertt425 I don't have any bandwidth to look into this but I'd suggest you could test it with that check removed e.g. change to:-

if (!isSeeking)

And see if there are any side effects.

[albertt425]

@peterfoot thanks, will do. I've actually already done that with my app (works fine), but I'll see if I can try some broader scenarios.

[ubik2]

There's an issue here on Android, when trying to have the IMediaElementController.Position property reflect the underlying FormsVideoView.Position, and the 300ms seek limit was likely introduced to make this work.

1. Getting MediaElement.Position triggers a call to set the MediaElement's IMediaElementController.Position = view.Position (in MediaElementRenderer).
2. Setting Controller.Position triggers the PositionChanged method (as part of the propertyChanged hook on the PositionProperty).
3. The PositionChanged method notices that the new position differs from the previous position by >300ms, and we're not already seeking, so it calls RequestSeek on the MediaElement.
4. The playback stutters, because of the seek.

With the 300ms limit, we can have a timer that runs every 200ms. That can fetch the MediaElement.Position, which will then be updated to reflect the FormsVideoView.Position, but will not trigger a Seek. As long as we continue to run it every 200ms while we're playing, we'll never trigger a Seek because we'll always have a close enough value.

I wouldn't have written the code this way myself, but the IMediaElementController interface is partly to blame. It has a Position setter, but doesn't have a way to indicate whether that setter is being called to reflect an existing change in the underlying view or whether that setter is being called to trigger a seek. This could be made to work if the MediaElement's IMediaElementController.Position property's getter accessed the underlying view's Position directly, in which case we could still maintain the idea that setting the IMediaElementController.Position should trigger a Seek. This doesn't match the BindableProperty model, though.

Edit: If you don't need to read the Position, you don't need this 300ms buffer, so for many users, this is sufficient. If you do want something like your own custom playback control, you probably need to read the Position, though.

[albertt425]

@ubik2 Thanks for the analysis. I ended up sticking with the older version of Xamarin Forms that has MediaElement, but it would be great to upgrade.

I've encountered this data binding model issue in other situations (sometimes controlling both ends, sometimes not). The separate interface looks like a reasonable workaround, although I wish there was a standard pattern for this.

In my case the video is paused (I am seeking to the next frame, which usually works depending on the underlying decoder). How about checking if the video is Playing and skip the 300ms check in Position.Set? If that sounds reasonable, I'll give it a spin. I'd prefer to avoid having my own fork of MediaElement.

var isDiscardableUpdate = (CurrentState == MediaElementState.Playing) && Math.Abs(value.Subtract(currentValue).TotalMilliseconds) <= 300; if (!isDiscardableUpdate && !isSeeking) RequestSeek(value);

I'd rather test whether the timer is running since that's more direct, but want to keep the changes minimal.

[ubik2]

@albertt425 that seems like a reasonable workaround. I imagine in your specific case, you could also do something like setting the position to 1seciond+1frame forward, then move 1second back in the SeekCompleted handler. It's a bit gross, but would let you use the unmodified MediaElement code.

In terms of the XCT repo, I just think the interface needs to be re-thought. There shouldn't be a property setter that is responsible for two completely different things (maintaining a variable to track the underlying position and changing the underlying position). Trying to guess which one was intended by the calling code is going to be a problem for such a common utility class that will be used by lots of developers in different situations.