Skip to content

Commit

Permalink
Fix crash when video URLs is empty (#799)
Browse files Browse the repository at this point in the history
* Check for empty video URLs

* Fix video downloading crash

* Fix video player crash
  • Loading branch information
ivan-magda authored Sep 29, 2020
1 parent c89a6c7 commit 27512c2
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 33 deletions.
23 changes: 15 additions & 8 deletions Stepic/Legacy/Controllers/VideoPlayer/Player.swift
Original file line number Diff line number Diff line change
Expand Up @@ -262,22 +262,29 @@ public class Player: UIViewController {
self.avplayer.removeTimeObserver(obs)
}

self.avplayer.removeTimeObserver(timeObserver!)
if self.timeObserver != nil {
self.avplayer.removeTimeObserver(timeObserver!)
}
self.delegate = nil

NotificationCenter.default.removeObserver(self)

self.playerView.layer.removeObserver(
self,
forKeyPath: PlayerReadyForDisplayKey,
context: &PlayerLayerObserverContext
)
self.playerView.player = nil
if self.playerView != nil {
self.playerView.layer.removeObserver(
self,
forKeyPath: PlayerReadyForDisplayKey,
context: &PlayerLayerObserverContext
)
self.playerView.player = nil
}

self.avplayer.removeObserver(self, forKeyPath: PlayerRateKey, context: &PlayerObserverContext)
if self.avplayer.observationInfo != nil {
self.avplayer.removeObserver(self, forKeyPath: PlayerRateKey, context: &PlayerObserverContext)
}

self.avplayer.pause()
self.setupPlayerItem(nil)

print("player is deinitialized")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,31 +227,35 @@ final class StepikVideoPlayerViewController: UIViewController {
override func viewDidLoad() {
super.viewDidLoad()

self.setupPlayer()
self.setupAppearance()
self.setupObservers()
self.setupGestureRecognizers()
self.addAccessibilitySupport()
if self.isVideoValid() {
self.setup()
}

self.analytics.send(.videoPlayerOpened)
}

override func viewDidAppear(_ animated: Bool) {
super.viewDidAppear(animated)

self.scheduleHidePlayerControlsTimer()
if self.isVideoValid() {
self.scheduleHidePlayerControlsTimer()

if !TooltipDefaultsManager.shared.didShowInVideoPlayer {
DispatchQueue.main.asyncAfter(deadline: .now() + 2) {
self.videoInBackgroundTooltip = TooltipFactory.videoInBackground
self.videoInBackgroundTooltip?.show(
direction: .down,
in: self.view,
from: self.fullscreenPlayButton,
isArrowVisible: false
)
TooltipDefaultsManager.shared.didShowInVideoPlayer = true
if !TooltipDefaultsManager.shared.didShowInVideoPlayer {
DispatchQueue.main.asyncAfter(deadline: .now() + 2) {
self.videoInBackgroundTooltip = TooltipFactory.videoInBackground
self.videoInBackgroundTooltip?.show(
direction: .down,
in: self.view,
from: self.fullscreenPlayButton,
isArrowVisible: false
)
TooltipDefaultsManager.shared.didShowInVideoPlayer = true
}
}
} else {
self.activityIndicator.isHidden = true
self.setPlayerBarControlsVisibleAnimated(visible: false)
self.displayInvalidVideoAlert()
}
}

Expand Down Expand Up @@ -283,7 +287,38 @@ final class StepikVideoPlayerViewController: UIViewController {
self.hidePlayerControlsTimer?.invalidate()
}

// MARK: Setup player
// MARK: Player Setup

private func isVideoValid() -> Bool {
self.video != nil && !self.video.urls.isEmpty && self.getInitialVideoQualityURL() != nil
}

private func displayInvalidVideoAlert() {
let alert = UIAlertController(
title: NSLocalizedString("VideoPlayerInvalidVideoAlertTitle", comment: ""),
message: NSLocalizedString("VideoPlayerInvalidVideoAlertMessage", comment: ""),
preferredStyle: .alert
)
alert.addAction(
UIAlertAction(
title: NSLocalizedString("OK", comment: ""),
style: .default,
handler: { [weak self] _ in
self?.dismissPlayer()
}
)
)

self.present(alert, animated: true)
}

private func setup() {
self.setupPlayer()
self.setupAppearance()
self.setupObservers()
self.setupGestureRecognizers()
self.addAccessibilitySupport()
}

private func setupAppearance() {
// Always adopt a light interface style.
Expand Down Expand Up @@ -622,11 +657,11 @@ final class StepikVideoPlayerViewController: UIViewController {
}
}

private func getInitialVideoQualityURL() -> URL {
private func getInitialVideoQualityURL() -> URL? {
if self.video.state == .cached {
return VideoStoredFileManager(
fileManager: FileManager.default
).getVideoStoredFile(videoID: video.id).require().localURL
).getVideoStoredFile(videoID: self.video.id)?.localURL
} else {
return self.video.getUrlForQuality(
self.streamVideoQualityStorageManager.globalStreamVideoQuality.uniqueIdentifier
Expand Down
10 changes: 7 additions & 3 deletions Stepic/Legacy/Model/Entities/Video/Video.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ final class Video: NSManagedObject, JSONSerializable {
return res
}

func getUrlForQuality(_ quality: String) -> URL {
func getUrlForQuality(_ quality: String) -> URL? {
if self.urls.isEmpty {
return nil
}

var urlToReturn: VideoURL?
var minDifference = 10000
for url in urls {
Expand All @@ -77,9 +81,9 @@ final class Video: NSManagedObject, JSONSerializable {
}

if let url = urlToReturn {
return URL(string: url.url)!
return URL(string: url.url)
} else {
return URL(string: urls[0].url)!
return URL(string: self.urls[0].url)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,11 @@ final class VideoDownloadingService: VideoDownloadingServiceProtocol {

let globalDownloadVideoQuality = self.downloadVideoQualityStorageManager.globalDownloadVideoQuality
let nearestQuality = video.getNearestQualityToDefault(globalDownloadVideoQuality.uniqueIdentifier)
let url = video.getUrlForQuality(nearestQuality)

guard let url = video.getUrlForQuality(nearestQuality) else {
throw Error.videoURLNotFound
}

let task = DownloaderTask(url: url)

self.setupReporters(for: task)
Expand Down Expand Up @@ -266,6 +270,7 @@ final class VideoDownloadingService: VideoDownloadingServiceProtocol {
case videoNotFound
case videoTemporaryFileNotSavedAsVideoFile
case videoDownloadingStopped
case videoURLNotFound
case alreadyDownloading
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ final class SyllabusDownloadsService: SyllabusDownloadsServiceProtocol {
private func startDownloading(section: Section? = nil, unit: Unit, steps: [Step]) throws {
let uncachedVideos = steps.compactMap { step -> Video? in
guard step.block.type == .video,
let video = step.block.video else {
let video = step.block.video,
!video.urls.isEmpty else {
return nil
}
return self.videoFileManager.getVideoStoredFile(videoID: video.id) == nil ? video : nil
Expand Down Expand Up @@ -471,7 +472,7 @@ final class SyllabusDownloadsService: SyllabusDownloadsServiceProtocol {

// Iterate through steps and determine final state
let stepsWithVideoCount = steps
.filter { $0.block.type == .video }
.filter { $0.block.type == .video && !($0.block.video?.urls.isEmpty ?? true) }
.count
let stepsWithImagesCount = steps
.filter { !$0.block.imageSourceURLs.isEmpty }
Expand Down
2 changes: 2 additions & 0 deletions Stepic/en.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,8 @@ VideoQuality = "Video quality";
Downloaded = "Downloaded";
VideoPlayerAutoplayPreferenceTitle = "Auto-transition to next step";
VideoPlayerAutoplayCancelTitle = "Stay on this step";
VideoPlayerInvalidVideoAlertTitle = "Playback error";
VideoPlayerInvalidVideoAlertMessage = "Video has not been uploaded or processed yet. Please, try again later.";

StepikSocialNetworksTitle = "Our social networks";

Expand Down
2 changes: 2 additions & 0 deletions Stepic/ru.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,8 @@ VideoRate = "Скорость воспроизведения";
VideoQuality = "Качество видео";
VideoPlayerAutoplayPreferenceTitle = "Автопереход на следующий шаг";
VideoPlayerAutoplayCancelTitle = "Остаться на этом шаге";
VideoPlayerInvalidVideoAlertTitle = "Ошибка воспроизведения";
VideoPlayerInvalidVideoAlertMessage = "Видео ещё не было загружено преподавателем или обработано. Пожалуйста, попробуйте воспроизвести видео позже.";

StepikSocialNetworksTitle = "Наши аккаунты в соцсетях";

Expand Down

0 comments on commit 27512c2

Please sign in to comment.