diff --git a/CHANGELOG.md b/CHANGELOG.md index 503a030204..34b2fb58cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ 8.17 ----- +- Add an optional "Rewind after interruptions" setting that jumps back a little when playback resumes after calls, alarms, navigation and other audio [#4788](https://github.com/Automattic/pocket-casts-ios/pull/4788) - Fix intro-skipped time not syncing to your account when it is the only listening stat that changed [#4755](https://github.com/Automattic/pocket-casts-ios/pull/4755) - Fix Up Next multi-select keeping episodes selected after they leave the queue, causing a wrong count and bulk actions on episodes no longer queued [#4709](https://github.com/Automattic/pocket-casts-ios/pull/4709) - Fix the Help & Feedback screen following the system/in-app dark theme and using incorrect colors in the navigation bar [#4698](https://github.com/Automattic/pocket-casts-ios/pull/4698) diff --git a/Modules/Sources/PocketCastsUtils/Feature Flags/FeatureFlag.swift b/Modules/Sources/PocketCastsUtils/Feature Flags/FeatureFlag.swift index 5ffdf2dbf9..ba93ea61de 100644 --- a/Modules/Sources/PocketCastsUtils/Feature Flags/FeatureFlag.swift +++ b/Modules/Sources/PocketCastsUtils/Feature Flags/FeatureFlag.swift @@ -320,6 +320,9 @@ public enum FeatureFlag: String, CaseIterable { /// Enable Smart Bookmarks case smartBookmarks + /// Enable the "Rewind after interruptions" setting: rewind playback after calls, alarms and other audio interruptions + case interruptionRewind + public var enabled: Bool { if let overriddenValue = FeatureFlagOverrideStore().overriddenValue(for: self) { return overriddenValue @@ -540,6 +543,8 @@ public enum FeatureFlag: String, CaseIterable { true case .smartBookmarks: BuildEnvironment.current == .debug + case .interruptionRewind: + BuildEnvironment.current == .debug } } diff --git a/PocketCastsTests/Tests/Playback/PlaybackCatchUpHelperTests.swift b/PocketCastsTests/Tests/Playback/PlaybackCatchUpHelperTests.swift new file mode 100644 index 0000000000..7fae9126e6 --- /dev/null +++ b/PocketCastsTests/Tests/Playback/PlaybackCatchUpHelperTests.swift @@ -0,0 +1,179 @@ +import XCTest + +@testable import PocketCastsDataModel +@testable import PocketCastsUtils +@testable import podcasts + +class PlaybackCatchUpHelperTests: XCTestCase { + private let helper = PlaybackCatchUpHelper() + + private let pauseTimeKey = "lastPauseTime" + private let pausedEpisodeUuidKey = "lastPausedEpisode" + private let pausedAtKey = "lastPausedAt" + private let pauseWasInterruptionKey = "lastPauseWasInterruption" + + override func setUpWithError() throws { + try super.setUpWithError() + + // start from a clean slate so leftovers from other tests or aborted runs can't leak in + removeHelperUserDefaults() + UserDefaults.standard.set(true, forKey: Constants.UserDefaults.intelligentPlaybackResumption) + try FeatureFlagOverrideStore().override(FeatureFlag.interruptionRewind, withValue: true) + } + + override func tearDown() { + removeHelperUserDefaults() + FeatureFlagOverrideStore().resetOverrides() + + super.tearDown() + } + + private func removeHelperUserDefaults() { + [pauseTimeKey, pausedEpisodeUuidKey, pausedAtKey, pauseWasInterruptionKey, + Constants.UserDefaults.intelligentPlaybackResumption, Constants.UserDefaults.interruptionRewindTime].forEach { + UserDefaults.standard.removeObject(forKey: $0) + } + } + + private func makeEpisode(playedUpTo: Double = 600) -> Episode { + let episode = EpisodeBuilder().with(playedUpTo: playedUpTo).build() + episode.uuid = "test-episode-uuid" + return episode + } + + private func setLastPauseTime(secondsAgo: TimeInterval) { + UserDefaults.standard.setValue(Date(timeIntervalSinceNow: -secondsAgo), forKey: pauseTimeKey) + } + + // MARK: - Interruption rewind + + func testInterruptionRewindAppliesAfterInterruption() { + let episode = makeEpisode() + Settings.interruptionRewindTime = 30 + + helper.playbackDidPause(of: episode, dueToInterruption: true) + + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo - 30) + } + + func testInterruptionRewindDefaultsToFiveSeconds() { + let episode = makeEpisode() + + helper.playbackDidPause(of: episode, dueToInterruption: true) + + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo - 5) + } + + func testInterruptionRewindDoesNotApplyWhenTurnedOff() { + let episode = makeEpisode() + Settings.interruptionRewindTime = 0 + + helper.playbackDidPause(of: episode, dueToInterruption: true) + + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo) + } + + func testInterruptionRewindDoesNotApplyWhenFeatureFlagDisabled() throws { + let episode = makeEpisode() + Settings.interruptionRewindTime = 30 + try FeatureFlagOverrideStore().override(FeatureFlag.interruptionRewind, withValue: false) + + helper.playbackDidPause(of: episode, dueToInterruption: true) + + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo) + } + + func testInterruptionRewindDoesNotApplyToRegularPauses() { + let episode = makeEpisode() + Settings.interruptionRewindTime = 30 + + helper.playbackDidPause(of: episode) + + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo) + } + + func testInterruptionRewindAppliesWhenIntelligentPlaybackResumptionIsOff() { + let episode = makeEpisode() + Settings.interruptionRewindTime = 30 + UserDefaults.standard.set(false, forKey: Constants.UserDefaults.intelligentPlaybackResumption) + + helper.playbackDidPause(of: episode, dueToInterruption: true) + + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo - 30) + } + + func testInterruptionRewindDoesNotRewindPastEpisodeStart() { + let episode = makeEpisode(playedUpTo: 3) + Settings.interruptionRewindTime = 30 + + helper.playbackDidPause(of: episode, dueToInterruption: true) + + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), 0) + } + + func testInterruptionRewindDoesNotApplyToADifferentEpisode() { + let episode = makeEpisode() + Settings.interruptionRewindTime = 30 + + helper.playbackDidPause(of: episode, dueToInterruption: true) + + let otherEpisode = makeEpisode() + otherEpisode.uuid = "another-episode-uuid" + + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: otherEpisode), otherEpisode.playedUpTo) + } + + // MARK: - Combining with the pause length rewind, the larger amount wins and they never stack + + func testLongerPauseLengthRewindWinsOverInterruptionRewind() { + let episode = makeEpisode() + Settings.interruptionRewindTime = 5 + + helper.playbackDidPause(of: episode, dueToInterruption: true) + setLastPauseTime(secondsAgo: 25.hours) + + // paused for more than 24 hours, the 30 second pause length rewind beats the 5 second interruption rewind + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo - 30) + } + + func testLongerInterruptionRewindWinsOverPauseLengthRewind() { + let episode = makeEpisode() + Settings.interruptionRewindTime = 60 + + helper.playbackDidPause(of: episode, dueToInterruption: true) + setLastPauseTime(secondsAgo: 6.minutes) + + // paused for more than 5 minutes, the 60 second interruption rewind beats the 10 second pause length rewind + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo - 60) + } + + func testRegularPauseAfterInterruptionClearsTheInterruptionRewind() { + let episode = makeEpisode() + Settings.interruptionRewindTime = 30 + + helper.playbackDidPause(of: episode, dueToInterruption: true) + helper.playbackDidPause(of: episode) + + // the most recent pause was a regular one, so the earlier interruption must not cause a rewind + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo) + } + + // MARK: - Existing pause length behaviour is unchanged + + func testPauseLengthRewindStillAppliesToRegularPauses() { + let episode = makeEpisode() + + helper.playbackDidPause(of: episode) + setLastPauseTime(secondsAgo: 6.minutes) + + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo - 10) + } + + func testNoRewindForShortRegularPause() { + let episode = makeEpisode() + + helper.playbackDidPause(of: episode) + + XCTAssertEqual(helper.adjustStartTimeIfNeeded(for: episode), episode.playedUpTo) + } +} diff --git a/podcasts/Analytics/AnalyticsEvent.swift b/podcasts/Analytics/AnalyticsEvent.swift index 87415bc304..d98ed3a377 100644 --- a/podcasts/Analytics/AnalyticsEvent.swift +++ b/podcasts/Analytics/AnalyticsEvent.swift @@ -646,6 +646,7 @@ enum AnalyticsEvent: String { case settingsGeneralOpenPlayerAutomaticallyToggled case settingsGeneralDisableLockScreenScrubberToggled case settingsGeneralIntelligentPlaybackToggled + case settingsGeneralInterruptionRewindChanged case settingsGeneralPlayUpNextOnTapToggled case settingsGeneralRemoteSkipsChaptersToggled case settingsGeneralAudioOnlyToggled diff --git a/podcasts/Constants.swift b/podcasts/Constants.swift index 8e239982f8..4b6722e1dc 100644 --- a/podcasts/Constants.swift +++ b/podcasts/Constants.swift @@ -119,6 +119,7 @@ struct Constants { static let keepScreenOnWhilePlaying = "SJKeepScreenOnWhenPlaying" static let openPlayerAutomatically = "SJOpenPlayerAutomatically" static let intelligentPlaybackResumption = "SJIntelligentPlaybackResumption" + static let interruptionRewindTime = "SJInterruptionRewindTime" static let hideImagesInShowNotes = "HideImagesInShowNotes" static let loadEmbeddedImages = "SJLoadEmbeddedArt" static let appBadge = "SJEppBadgeShows" @@ -276,6 +277,8 @@ struct Constants { static let tableSectionHeaderHeight: CGFloat = 38 static let tableRowHeaderHeight: CGFloat = 64 + static let defaultInterruptionRewindTime = 5 + static let refreshTaskId = "au.com.shiftyjelly.podcasts.Refresh" /// We show the offer by default since if the app was just downloaded diff --git a/podcasts/GeneralSettingsViewController.swift b/podcasts/GeneralSettingsViewController.swift index de5dfe3b72..f163ef8b1d 100644 --- a/podcasts/GeneralSettingsViewController.swift +++ b/podcasts/GeneralSettingsViewController.swift @@ -12,12 +12,16 @@ class GeneralSettingsViewController: PCViewController, UITableViewDelegate, UITa let debounce = Debounce(delay: Constants.defaultDebounceTime) - enum TableRow { case skipForward, skipBack, keepScreenAwake, openPlayer, intelligentPlaybackResumption, defaultRowAction, extraMediaActions, defaultAddToUpNextSwipe, defaultGrouping, defaultArchive, playUpNextOnTap, legacyBluetooth, multiSelectGesture, openLinksInBrowser, publishChapterTitles, generatedChapters, autoplay, autoRestartSleepTimer, shakeToRestartSleepTimer, isLockScreenScrubberDisabled, voiceBoostN, audioOnly } + enum TableRow { case skipForward, skipBack, keepScreenAwake, openPlayer, intelligentPlaybackResumption, interruptionRewind, defaultRowAction, extraMediaActions, defaultAddToUpNextSwipe, defaultGrouping, defaultArchive, playUpNextOnTap, legacyBluetooth, multiSelectGesture, openLinksInBrowser, publishChapterTitles, generatedChapters, autoplay, autoRestartSleepTimer, shakeToRestartSleepTimer, isLockScreenScrubberDisabled, voiceBoostN, audioOnly } private var tableData: [[TableRow]] { var data: [[TableRow]] = [[.defaultRowAction, .defaultGrouping, .defaultArchive, .defaultAddToUpNextSwipe, .openLinksInBrowser], [.skipForward, .skipBack, .keepScreenAwake, .openPlayer, .isLockScreenScrubberDisabled, .intelligentPlaybackResumption], [.autoRestartSleepTimer], [.shakeToRestartSleepTimer], [.playUpNextOnTap], [.extraMediaActions], [.legacyBluetooth], [.multiSelectGesture], [.publishChapterTitles], [.autoplay]] if FeatureFlag.hls.enabled { data.insert([.audioOnly], at: 2) } + // its own section (directly below the player section) so both it and intelligent playback resumption keep their footer descriptions + if FeatureFlag.interruptionRewind.enabled { + data.insert([.interruptionRewind], at: 2) + } if FeatureFlag.generatedChapters.enabled { data.append([.generatedChapters]) } @@ -187,6 +191,13 @@ class GeneralSettingsViewController: PCViewController, UITableViewDelegate, UITa cell.cellSwitch.removeTarget(self, action: nil, for: .valueChanged) cell.cellSwitch.addTarget(self, action: #selector(intelligentPlaybackResumptionToggled(_:)), for: .valueChanged) + return cell + case .interruptionRewind: + let cell = tableView.dequeueReusableCell(withIdentifier: disclosureCellId, for: indexPath) as! DisclosureCell + cell.cellLabel.text = L10n.settingsGeneralInterruptionRewind + let rewindTime = Settings.interruptionRewindTime + cell.cellSecondaryLabel.text = rewindTime > 0 ? L10n.timeShorthand(rewindTime) : L10n.off + return cell case .defaultRowAction: let cell = tableView.dequeueReusableCell(withIdentifier: disclosureCellId, for: indexPath) as! DisclosureCell @@ -445,6 +456,20 @@ class GeneralSettingsViewController: PCViewController, UITableViewDelegate, UITa } options.addAction(action: playLastAction) options.present(from: self) + } else if row == .interruptionRewind { + let currentTime = Settings.interruptionRewindTime + + let options = OptionsPicker(title: L10n.settingsGeneralInterruptionRewind) + for seconds in [0, 5, 10, 15, 30, 60] { + let label = seconds > 0 ? L10n.timeShorthand(seconds) : L10n.off + let action = OptionAction(label: label, selected: currentTime == seconds) { + Settings.interruptionRewindTime = seconds + tableView.reloadData() + Settings.trackValueChanged(.settingsGeneralInterruptionRewindChanged, value: seconds) + } + options.addAction(action: action) + } + options.present(from: self) } } @@ -467,6 +492,8 @@ class GeneralSettingsViewController: PCViewController, UITableViewDelegate, UITa switch lastSectionItem { case .intelligentPlaybackResumption: return L10n.settingsGeneralSmartPlaybackSubtitle + case .interruptionRewind: + return L10n.settingsGeneralInterruptionRewindSubtitle case .playUpNextOnTap: return Settings.playUpNextOnTap() ? L10n.settingsGeneralUpNextTapOnSubtitle : L10n.settingsGeneralUpNextTapOffSubtitle case .extraMediaActions: diff --git a/podcasts/PlaybackCatchUpHelper.swift b/podcasts/PlaybackCatchUpHelper.swift index 0440816812..3fa83d6a42 100644 --- a/podcasts/PlaybackCatchUpHelper.swift +++ b/podcasts/PlaybackCatchUpHelper.swift @@ -10,32 +10,53 @@ struct PlaybackCatchUpHelper { return episode.playedUpTo #else // if it's a different episode, or not still at the time it was at when it was last paused, just play from where it's up to - let intelligentPlaybackResumption = UserDefaults.standard.bool(forKey: Constants.UserDefaults.intelligentPlaybackResumption) - if !intelligentPlaybackResumption || episode.uuid != lastPausedEpisodeUuid() || episode.playedUpTo != lastPausedAt() { return episode.playedUpTo } + if episode.uuid != lastPausedEpisodeUuid() || episode.playedUpTo != lastPausedAt() { return episode.playedUpTo } + + // the pause length rewind and the interruption rewind can both apply to the same resume, take the larger of the two rather than stacking them + let rewindAmount = max(pauseLengthRewindAmount(), interruptionRewindAmount()) + if rewindAmount <= 0 { return episode.playedUpTo } - guard let lastPauseTime = lastPauseTime() else { return episode.playedUpTo } + return max(0, episode.playedUpTo - rewindAmount) + #endif + } + + func playbackDidPause(of episode: BaseEpisode, dueToInterruption: Bool = false) { + setLastPauseTimeToNow() + setLastPausedEpisodeUuid(episode.uuid) + setLastPausedAt(episode.playedUpTo) + setLastPauseWasInterruption(dueToInterruption) + } + + #if !os(watchOS) + private func pauseLengthRewindAmount() -> TimeInterval { + let intelligentPlaybackResumption = UserDefaults.standard.bool(forKey: Constants.UserDefaults.intelligentPlaybackResumption) + guard intelligentPlaybackResumption, let lastPauseTime = lastPauseTime() else { return 0 } if DateUtil.hasEnoughTimePassed(since: lastPauseTime, time: 24.hours) { FileLog.shared.addMessage("More than 24 hours since this episode was paused, jumping back 30 seconds") - return max(0, episode.playedUpTo - 30.seconds) + return 30.seconds } else if DateUtil.hasEnoughTimePassed(since: lastPauseTime, time: 1.hour) { FileLog.shared.addMessage("More than 1 hour since this episode was paused, jumping back 15 seconds") - return max(0, episode.playedUpTo - 15.seconds) + return 15.seconds } else if DateUtil.hasEnoughTimePassed(since: lastPauseTime, time: 5.minutes) { FileLog.shared.addMessage("More than 5 minutes since this episode was paused, jumping back 10 seconds") - return max(0, episode.playedUpTo - 10.seconds) + return 10.seconds } FileLog.shared.addMessage("Not enough time passed since this episode was last paused, no time adjustment required") - return episode.playedUpTo - #endif - } + return 0 + } - func playbackDidPause(of episode: BaseEpisode) { - setLastPauseTimeToNow() - setLastPausedEpisodeUuid(episode.uuid) - setLastPausedAt(episode.playedUpTo) - } + private func interruptionRewindAmount() -> TimeInterval { + guard FeatureFlag.interruptionRewind.enabled, lastPauseWasInterruption() else { return 0 } + + let rewindTime = UserDefaults.standard.object(forKey: Constants.UserDefaults.interruptionRewindTime) as? Int ?? Constants.Values.defaultInterruptionRewindTime + guard rewindTime > 0 else { return 0 } + + FileLog.shared.addMessage("Playback was interrupted, jumping back \(rewindTime) seconds") + return TimeInterval(rewindTime) + } + #endif // MARK: - Pause Time @@ -63,6 +84,17 @@ struct PlaybackCatchUpHelper { UserDefaults.standard.setValue(uuid, forKey: pausedEpisodeUuidKey) } + // MARK: - Interruption + + private let pauseWasInterruptionKey = "lastPauseWasInterruption" + private func lastPauseWasInterruption() -> Bool { + UserDefaults.standard.bool(forKey: pauseWasInterruptionKey) + } + + private func setLastPauseWasInterruption(_ wasInterruption: Bool) { + UserDefaults.standard.setValue(wasInterruption, forKey: pauseWasInterruptionKey) + } + // MARK: - Paused At private let pausedAtKey = "lastPausedAt" diff --git a/podcasts/PlaybackManager.swift b/podcasts/PlaybackManager.swift index 5afd68c400..8cabb1f82e 100644 --- a/podcasts/PlaybackManager.swift +++ b/podcasts/PlaybackManager.swift @@ -2348,7 +2348,15 @@ class PlaybackManager: ServerPlaybackDelegate { } if let episode = currentEpisode() { - catchUpHelper.playbackDidPause(of: episode) + // record the frozen player position and stop the progress timer before storing the pause, + // otherwise the timer keeps moving playedUpTo and the catch up helper's position guard + // rejects the adjustment when playback resumes + recordPlaybackPosition(sendToServerImmediately: false, fireNotifications: false) + cancelUpdateTimer() + + // interruptInProgress is false here when this began event is a route disconnection being ignored, + // those aren't real interruptions so they shouldn't trigger the interruption rewind either + catchUpHelper.playbackDidPause(of: episode, dueToInterruption: interruptInProgress && wasPlayingBeforeInterruption) } NotificationCenter.postOnMainThread(notification: Constants.Notifications.playbackPaused) } diff --git a/podcasts/Settings.swift b/podcasts/Settings.swift index 37baf6ec67..7fcb375aa1 100644 --- a/podcasts/Settings.swift +++ b/podcasts/Settings.swift @@ -1078,6 +1078,17 @@ class Settings: NSObject { } } + /// How many seconds to rewind when playback resumes after an audio interruption, 0 means off. + /// Stored on device only, not synced. + static var interruptionRewindTime: Int { + get { + UserDefaults.standard.object(forKey: Constants.UserDefaults.interruptionRewindTime) as? Int ?? Constants.Values.defaultInterruptionRewindTime + } + set { + UserDefaults.standard.set(newValue, forKey: Constants.UserDefaults.interruptionRewindTime) + } + } + static var skipForwardTime: Int { get { ServerSettings.skipForwardTime() diff --git a/podcasts/en.lproj/Localizable.strings b/podcasts/en.lproj/Localizable.strings index dc61f97fb5..2488b2540f 100644 --- a/podcasts/en.lproj/Localizable.strings +++ b/podcasts/en.lproj/Localizable.strings @@ -2713,6 +2713,12 @@ /* Subtitle explaining the feature that adjusts the playback position when resuming. */ "settings_general_smart_playback_subtitle" = "If on, Pocket Casts will go back a little in episodes you resume so you can catch up more comfortably."; +/* Setting to choose how many seconds playback rewinds after an audio interruption. */ +"settings_general_interruption_rewind" = "Rewind after interruptions"; + +/* Subtitle explaining the setting that rewinds playback after an audio interruption. */ +"settings_general_interruption_rewind_subtitle" = "Go back a little after playback is interrupted by calls, alarms, navigation and other audio."; + /* Setting toggle to enable the feature that disables the lock screen scrubber. */ "settings_general_lock_screen_disabled" = "Enable Lock Screen Scrubbing";