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

VoiceBroadcast: Fix chunk processing #7113

Merged
merged 2 commits into from
Nov 29, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic
private var isPlaybackInitialized: Bool = false
private var acceptProgressUpdates: Bool = true
private var isActuallyPaused: Bool = false
private var isProcessingVoiceBroadcastChunk: Bool = false
private var reloadVoiceBroadcastChunkQueue: Bool = false
private var seekToChunkTime: TimeInterval?

private var isPlayingLastChunk: Bool {
let chunks = reorderVoiceBroadcastChunks(chunks: Array(voiceBroadcastAggregator.voiceBroadcast.chunks))
Expand Down Expand Up @@ -162,9 +165,9 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic
// MARK: - Voice broadcast chunks playback

/// Start the playback from the beginning or push more chunks to it
private func processPendingVoiceBroadcastChunks(_ time: TimeInterval? = nil) {
private func processPendingVoiceBroadcastChunks() {
reorderPendingVoiceBroadcastChunks()
processNextVoiceBroadcastChunk(time)
processNextVoiceBroadcastChunk()
}

/// Start the playback from the last known chunk
Expand All @@ -185,7 +188,7 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic
chunks.sorted(by: {$0.sequence < $1.sequence})
}

private func processNextVoiceBroadcastChunk(_ time: TimeInterval? = nil) {
private func processNextVoiceBroadcastChunk() {
MXLog.debug("[VoiceBroadcastPlaybackViewModel] processNextVoiceBroadcastChunk: \(voiceBroadcastChunkQueue.count) chunks remaining")

guard voiceBroadcastChunkQueue.count > 0 else {
Expand All @@ -197,6 +200,13 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic
state.playbackState = .buffering
}

guard !isProcessingVoiceBroadcastChunk else {
// Chunks caching is already in progress
return
}

isProcessingVoiceBroadcastChunk = true

// TODO: Control the download rate to avoid to download all chunk in mass
// We could synchronise it with the number of chunks in the player playlist (audioPlayer.playerItems)

Expand All @@ -208,9 +218,12 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic
return
}

// TODO: Make sure there has no new incoming chunk that should be before this attachment
// Be careful that this new chunk is not older than the chunk being played by the audio player. Else
// we will get an unexecpted rewind.
self.isProcessingVoiceBroadcastChunk = false
if self.reloadVoiceBroadcastChunkQueue, self.seekToChunkTime != nil {
Copy link
Member

Choose a reason for hiding this comment

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

@yostyle why do you check seekToChunkTime here?

self.reloadVoiceBroadcastChunkQueue = false
self.processNextVoiceBroadcastChunk()
return
}

switch result {
case .success(let result):
Expand All @@ -224,8 +237,9 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic
// Append the chunk to the current playlist
audioPlayer.addContentFromURL(result.url)

if let time = time {
if let time = self.seekToChunkTime {
audioPlayer.seekToTime(time)
self.seekToChunkTime = nil
}

// Resume the player. Needed after a buffering
Expand All @@ -247,8 +261,9 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic
audioPlayer.loadContentFromURL(result.url, displayName: chunk.attachment.originalFileName)
self.displayLink.isPaused = false
audioPlayer.play()
if let time = time {
if let time = self.seekToChunkTime {
audioPlayer.seekToTime(time)
self.seekToChunkTime = nil
}
self.audioPlayer = audioPlayer
}
Expand Down Expand Up @@ -281,7 +296,9 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic
audioPlayer?.pause()
displayLink.isPaused = true
} else {
// Flush the current audio player playlist
// Flush the chunks queue and the current audio player playlist
voiceBroadcastChunkQueue = []
reloadVoiceBroadcastChunkQueue = true
Copy link
Member

Choose a reason for hiding this comment

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

@yostyle you should turn on this flag only when isProcessingVoiceBroadcastChunk is true
Don't you think so?

audioPlayer?.removeAllPlayerItems()

let chunks = reorderVoiceBroadcastChunks(chunks: Array(voiceBroadcastAggregator.voiceBroadcast.chunks))
Expand All @@ -299,7 +316,8 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic

MXLog.debug("[VoiceBroadcastPlaybackViewModel] didSliderChanged: restart to time: \(state.bindings.progress) milliseconds")
let time = state.bindings.progress - state.playingState.duration + Float(chunksDuration)
processPendingVoiceBroadcastChunks(TimeInterval(time / 1000))
seekToChunkTime = TimeInterval(time / 1000)
processPendingVoiceBroadcastChunks()
}
}

Expand Down