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

feat: save playback speed #67

Merged
merged 7 commits into from
Aug 22, 2024
Merged

feat: save playback speed #67

merged 7 commits into from
Aug 22, 2024

Conversation

rnr
Copy link
Collaborator

@rnr rnr commented Aug 12, 2024

This PR adds playback speed preservation for native videos. Also it adds fullscreen mode for YouTube videos for iPad
From doc

Native videos > The Playback speed shifts back to normal after going back and resuming the course video
RPReplay_Final1721741975.MP4

@rnr rnr requested a review from forgotvas August 12, 2024 12:31
@@ -84,6 +85,19 @@ public class PlayerViewControllerHolder: PlayerViewControllerHolderProtocol {
playerController.delegate = playerDelegate
playerController.player = playerTracker.player as? AVPlayer
playerController.player?.currentItem?.preferredMaximumResolution = videoResolution

let storage = Container.shared.resolve(CourseStorage.self)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make it as injection

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -11,6 +11,7 @@ import Core
public protocol CourseStorage {
var allowedDownloadLargeFile: Bool? { get set }
var userSettings: UserSettings? { get set }
var videoPlaybackSpeed: Float? {get set}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better to move it to UserSettings where we have streamingQuality option?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a separate setting for this at the profile/settings level. It can only be changed in the video player. I don't think we need to mix this with the settings object. Thank you

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we could group all the user settings regardless of their place of usage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. please re-check

@rnr rnr requested a review from saeedbashir August 12, 2024 15:35
@rnr rnr requested a review from forgotvas August 16, 2024 09:40
self.pipManager = pipManager
self.playerTracker = playerTracker
self.playerService = playerService
let youtubePlayer = playerTracker.player as? YouTubePlayer
var configuration = youtubePlayer?.configuration
configuration?.autoPlay = !pipManager.isPipActive
configuration?.fullscreenMode = .web

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rnr Is this change relevant?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saeedbashir it's not relevant to playback speed. It adds fullscreen for iPad. Update PR description.
Thank you

@rnr rnr merged commit f903937 into 2U/develop Aug 22, 2024
3 checks passed
@rnr rnr deleted the save-playback-speed branch August 22, 2024 09:16
saeedbashir pushed a commit that referenced this pull request Aug 23, 2024
* chore: save playback speed in CourseStorage

* chore: add full screen for youtube on iPad

* chore: save value in userdefaults

* chore: little refactor

* fix: fix warning

* chore: added CourseStorage as injection

* chore: moved playback speed to user settings

---------

Co-authored-by: Anton Yarmolenko <[email protected]>
rnr added a commit that referenced this pull request Dec 5, 2024
* chore: save playback speed in CourseStorage

* chore: add full screen for youtube on iPad

* chore: save value in userdefaults

* chore: little refactor

* fix: fix warning

* chore: added CourseStorage as injection

* chore: moved playback speed to user settings

---------

Co-authored-by: Anton Yarmolenko <[email protected]>
rnr added a commit that referenced this pull request Dec 9, 2024
* fix: video tab with youtube (#52)

* fix: don't show error if only youtube videos

* fix: call UI updates from main queue

* fix: don't send completion at youtube video start

---------

Co-authored-by: Anton Yarmolenko <[email protected]>

* chore: delete unneeded MainActor instructions

* feat: save playback speed (#67)

* chore: save playback speed in CourseStorage

* chore: add full screen for youtube on iPad

* chore: save value in userdefaults

* chore: little refactor

* fix: fix warning

* chore: added CourseStorage as injection

* chore: moved playback speed to user settings

---------

Co-authored-by: Anton Yarmolenko <[email protected]>

* chore: analytics screen events improvements and name fixes (#76)

* chore: fix tests

---------

Co-authored-by: Anton Yarmolenko <[email protected]>
Co-authored-by: Saeed Bashir <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants