Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix HLS CTD on Windows #1543

Merged
merged 8 commits into from
Nov 27, 2023

Conversation

ne0rrmatrix
Copy link
Contributor

  • Bug fix for HLS CTD on Windows

Description of Change

Reset position and duration when switching sources

Linked Issues

Issue: #1538

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Additional information

This issue is not directly an issue with media element as it is the slider that crashes. This PR fixes that issue. I checked android to see if the position and duration is reset when changing sources. It is not. It reset either on opening or when media fails. Do we want to go back and change this in the future? If we do we should check every device for this issue. Or is this intended behavior and only an issue in windows due to slider behavior on that platform?

This might give some insight into slider behavior and a current issue being tracked with it. dotnet/maui#12285

@ne0rrmatrix
Copy link
Contributor Author

@pictos btw this fixes a crash when media element source is invalid for windows. It has an exception that is thrown by Maui in sample app with:

void MediaElement_PropertyChanged(object? sender, PropertyChangedEventArgs e)
{
	if (e.PropertyName == MediaElement.DurationProperty.PropertyName)
	{
		logger.LogInformation("Duration: {newDuration}", MediaElement.Duration);
		PositionSlider.Maximum = MediaElement.Duration.TotalSeconds;
	}
}

//here => PositionSlider.Maximum = MediaElement.Duration.TotalSeconds;
void OnPositionChanged(object? sender, MediaPositionChangedEventArgs e)
{
	logger.LogInformation("Position changed to {position}", e.Position);
	PositionSlider.Value = e.Position.TotalSeconds;
}
// here => PositionSlider.Value = e.Position.TotalSeconds;

That throws exception before code changes. After if the HLS stream url is not valid it just fails gracefully.

@pictos
Copy link
Member

pictos commented Nov 27, 2023

@ne0rrmatrix what do you mean by "After if the HLS stream url is not valid it just fails gracefully.", it will log some info or will throw an exception that end dev can catch?

@ne0rrmatrix
Copy link
Contributor Author

ne0rrmatrix commented Nov 27, 2023

@pictos this is what it shows currently in windows as the current source for media element HLS is not valid atm anyways. So, yes to your question. It does show a very obvious error that both user and dev can see. It is built into media player for windows and does not crash the app.

image

@jfversluis jfversluis added the 📽️ MediaElement Issue/PR that has to do with MediaElement label Nov 27, 2023
@jfversluis jfversluis enabled auto-merge (squash) November 27, 2023 14:36
@jfversluis jfversluis merged commit 69cb515 into CommunityToolkit:main Nov 27, 2023
7 checks passed
@ne0rrmatrix ne0rrmatrix deleted the FixMediaElementHLS branch February 12, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📽️ MediaElement Issue/PR that has to do with MediaElement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MediaElement Sample loadHLS exception [BUG]
3 participants