Fix fatal error, video info not available, #3013 #3434
Conversation
Change PlayerWindowController.updatePlayTime to not report a fatal error if the position or duration is not known as can occur when the function is called while mpv is still loading the file.
The fix in PR iina#3434 MUST be merged before merging this commit.. This commit will: - Change MainWindowController.hideUI to stop the timer that updates the OSC - Change MainWindowController.showUI to sync the UI and start the timer - Change PlaybackInfo.constrainVideoPosition to check that`videoPosition is not nil
iina/PlayerWindowController.swift
Outdated
| Logger.fatal("video info not available") | ||
| } | ||
| // IINA listens for changes to mpv properties such as chapter that can occur during file loading | ||
| // resulting in this function being called before mpv has set its poistion and duration |
There was a problem hiding this comment.
| // resulting in this function being called before mpv has set its poistion and duration | |
| // resulting in this function being called before mpv has set its position and duration |
iina/PlayerWindowController.swift
Outdated
| // IINA listens for changes to mpv properties such as chapter that can occur during file loading | ||
| // resulting in this function being called before mpv has set its poistion and duration | ||
| // properties. If position or duration are unknown assume the file is still being loaded. | ||
| guard loaded, let duration = player.info.videoDuration, let pos = player.info.videoPosition else { return } |
There was a problem hiding this comment.
Perhaps we could just downgrade the logging to debug?
There was a problem hiding this comment.
Good point. See what you think of the new commit I just pushed.
There was a problem hiding this comment.
I wouldn't actually mind just having one log statement–my understanding is that both of these should be available simultaneously. Am I missing something, or perhaps you feel that they should be separate?
There was a problem hiding this comment.
According to the mpv manual, mpv does not guarantee that it is able to determine the duration. This is mentioned in several places in the manual. This is from the property definition:
Duration of the current file in seconds. If the duration is unknown, the property is unavailable. Note that the file duration is not always exactly known, so this is an estimate.
I have not encountered a case where mpv did not provide a value for duration, but I don't do that much streaming. Probably happens with live streams? I figured it was best to adhere to the documented behavior and not assume the duration is known.
For that reason I thought it was important in the log to distinguish between which piece of video information is missing. Of course there is more than one way to do that.
The commit addresses a review comment that logging should be preserved. The commit also fixes a typo in a comment.
* Fix fatal error, video info not available, #3013 Change PlayerWindowController.updatePlayTime to not report a fatal error if the position or duration is not known as can occur when the function is called while mpv is still loading the file. * Fix fatal error, video info not available, #3013 The commit addresses a review comment that logging should be preserved. The commit also fixes a typo in a comment.
The fix in PR #3434 MUST be merged before merging this commit.. This commit will: - Change MainWindowController.hideUI to stop the timer that updates the OSC - Change MainWindowController.showUI to sync the UI and start the timer - Change PlaybackInfo.constrainVideoPosition to check that`videoPosition is not nil
Change PlayerWindowController.updatePlayTime to not report a fatal error
if the position or duration is not known as can occur when the function
is called while mpv is still loading the file.
Description: