-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[video_player] Fixed HLS streams on iOS (#48760) #3048
Conversation
75ccb62 to
007ed0b
Compare
|
Rebase with master and resolve conflicting version number |
007ed0b to
584d822
Compare
@mvanbeusekom Done |
amirh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! left a few comments
| children: <Widget>[ | ||
| LinearProgressIndicator( | ||
| value: maxBuffering / duration, | ||
| value: duration > 0 ? maxBuffering / duration : 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to cover the crashy behavior with a test?
| children: <Widget>[ | ||
| LinearProgressIndicator( | ||
| value: maxBuffering / duration, | ||
| value: duration > 0 ? maxBuffering / duration : 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does duration 0 means it's indefinite? if so should we use null for value to show an indefinite progress indicator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's 0 on iOS. Android returns different values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have an indication on Android that it's indefinite? what does the progress bar show for indefinite streams on Android?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exoPlayer does have a isDynamic function on the Window. This is set to true when a Live stream is being played. I believe this would be the best solution. Link to the docs..
I could implement a VideoPlayerController().isDurationIndefinite getter that returns true when ExoPlayer returns true on the isDynamic getter on Android, and when isDurationIndefinite returns true on iOS?
|
|
||
| // The player may be initialized but still needs to determine the duration. | ||
| if ([self duration] == 0) { | ||
| if ([self duration] == 0 && ![self isDurationIndefinite]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cyanglaz any hints on how to test cover this?
Description
Implements the changes of #1743 with the desired changes to the implementation. This makes it possible (again) to stream HLS stream with an indefinite length.
On an iPhone simulator I only hear the audio when playing a HLS stream, but running it on an iPad everything works.
I added some checks to the VideoProgressIndicator. On iOS, because the duration is 0, it's going to divide 0 by 0, which results in a crash. This is not an issue in Android. On iOS the ProgressIndicator now just shows no progress, which I believe should be the desired behaviour.
Related Issues
#1743
Fixes #48670
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?