diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d7c0f345..545587b1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Fix case where shuffled collection progress would reset early - This only happened when also grouping episodes (e.g. `Keep Multi-Part Episodes Together`) - More groups made it more likely to happen +- Fix marathons and playlists that would never pick a new order + - `Marathon Shuffle Groups` and `Shuffle Playlist Items` choose a new order every time everything has played once + - Content that can never play was still counted as waiting to play, so that never happened and the order stayed the same forever + - This was caused by specials (season 0), which are skipped by `Season, Episode` order; marathons grouped by show or season use that order unless `Marathon Shuffle Items` is enabled + - Affected playouts also became slower to build the longer they ran +- Fix shuffled playlists and marathon groups changing order when a playout is rebuilt + - Existing playlists and marathons will change order once after updating +- Fix playlists used as filler skipping content + - When something doesn't fit, ETV puts the filler back where it was and tries again, but playlists were only partly put back + - This could skip playlist content, and could cause the next playout build to resume in the wrong place - Fix green line sometimes seen with NVIDIA and AMD/VAAPI encoding - Both bugs were in ffmpeg, and ETV's patched ffmpeg 8.1.2 is required for the fixes diff --git a/ErsatzTV.Core.Tests/Scheduling/MarathonEnumeratorTests.cs b/ErsatzTV.Core.Tests/Scheduling/MarathonEnumeratorTests.cs new file mode 100644 index 000000000..e8be029ce --- /dev/null +++ b/ErsatzTV.Core.Tests/Scheduling/MarathonEnumeratorTests.cs @@ -0,0 +1,190 @@ +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Repositories; +using ErsatzTV.Core.Scheduling; +using ErsatzTV.Core.Scheduling.Engine; +using NSubstitute; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Core.Tests.Scheduling; + +/// +/// Marathon content builds a over one playlist item per group, +/// so it inherits that enumerator's save/restore behaviour. These cover the state handling, not +/// the grouping. +/// +[TestFixture] +public class MarathonEnumeratorTests +{ + private const int Seed = 987654321; + + [SetUp] + public void SetUp() => _cancellationToken = new CancellationTokenSource(TimeSpan.FromSeconds(10)).Token; + + private CancellationToken _cancellationToken; + + /// + /// A cycle completes when every media item has been played, which resets the index to 0 and + /// reshuffles the groups. Two shows of three episodes each is six items, so the index has to + /// come back to 0 within six moves. + /// + [Test] + public async Task Marathon_Should_Complete_A_Cycle() + { + List mediaItems = + [ + .. Show(showId: 1, seasonNumber: 1, episodes: 3, firstId: 10), + .. Show(showId: 2, seasonNumber: 1, episodes: 3, firstId: 20) + ]; + + PlaylistEnumerator enumerator = await CreateMarathon(mediaItems); + + List indexes = Advance(enumerator, 12); + + indexes.ShouldContain(0, "the index never returned to 0, so the cycle never completed"); + } + + /// + /// Season 0 episodes are filtered out by the season/episode enumerator but still counted as + /// items the cycle is waiting on, so the cycle can never complete. + /// + [Test] + public async Task Marathon_Should_Complete_A_Cycle_With_Specials() + { + List mediaItems = + [ + .. Show(showId: 1, seasonNumber: 1, episodes: 3, firstId: 10), + .. Show(showId: 1, seasonNumber: 0, episodes: 1, firstId: 15), + .. Show(showId: 2, seasonNumber: 1, episodes: 3, firstId: 20) + ]; + + PlaylistEnumerator enumerator = await CreateMarathon(mediaItems); + + List indexes = Advance(enumerator, 60); + + indexes.ShouldContain(0, "the index never returned to 0, so the cycle never completed"); + } + + /// + /// The flood and duration schedulers snapshot every enumerator's state before trying an item + /// and hand it back when the item doesn't fit, so ResetState has to actually rewind. + /// + [Test] + public async Task Marathon_Should_Rewind_On_ResetState() + { + List mediaItems = + [ + .. Show(showId: 1, seasonNumber: 1, episodes: 3, firstId: 10), + .. Show(showId: 2, seasonNumber: 1, episodes: 3, firstId: 20) + ]; + + PlaylistEnumerator enumerator = await CreateMarathon(mediaItems); + + + enumerator.MoveNext(Option.None); + + CollectionEnumeratorState snapshot = enumerator.State.Clone(); + int expected = enumerator.Current.Map(mi => mi.Id).IfNone(-1); + + enumerator.MoveNext(Option.None); + enumerator.MoveNext(Option.None); + + enumerator.ResetState(snapshot); + + enumerator.Current.Map(mi => mi.Id).IfNone(-1).ShouldBe(expected); + } + + /// + /// Rebuilding a playout restores the enumerator from the persisted state, which has to land on + /// the same item the previous build stopped at. + /// + [Test] + public async Task Marathon_Should_Resume_Where_It_Left_Off() + { + List mediaItems = + [ + .. Show(showId: 1, seasonNumber: 1, episodes: 3, firstId: 10), + .. Show(showId: 2, seasonNumber: 1, episodes: 3, firstId: 20) + ]; + + PlaylistEnumerator reference = await CreateMarathon(mediaItems); + + var sequence = new List(); + var states = new List(); + for (var i = 0; i < 12; i++) + { + states.Add(reference.State.Clone()); + sequence.Add(reference.Current.Map(mi => mi.Id).IfNone(-1)); + reference.MoveNext(Option.None); + } + + var mismatches = new List(); + for (var i = 0; i < states.Count; i++) + { + PlaylistEnumerator resumed = await CreateMarathon(mediaItems, states[i]); + int actual = resumed.Current.Map(mi => mi.Id).IfNone(-1); + if (actual != sequence[i]) + { + mismatches.Add($"position {i} (index {states[i].Index}): expected {sequence[i]}, got {actual}"); + } + } + + mismatches.ShouldBeEmpty(string.Join("; ", mismatches)); + } + + private async Task CreateMarathon( + List mediaItems, + CollectionEnumeratorState state = null) + { + IMediaCollectionRepository repo = Substitute.For(); + var helper = new MarathonHelper(repo); + + Option maybeEnumerator = await helper.GetEnumerator( + mediaItems, + MarathonGroupBy.Show, + marathonShuffleGroups: false, + marathonShuffleItems: false, + marathonBatchSize: Option.None, + state ?? new CollectionEnumeratorState { Seed = Seed, Index = 0 }, + _cancellationToken); + + return maybeEnumerator.IfNone(() => throw new InvalidOperationException("no marathon enumerator")); + } + + private static List Advance(PlaylistEnumerator enumerator, int count) + { + var indexes = new List(); + for (var i = 0; i < count; i++) + { + enumerator.MoveNext(Option.None); + indexes.Add(enumerator.State.Index); + } + + return indexes; + } + + private static List Show(int showId, int seasonNumber, int episodes, int firstId) + { + var seasonId = showId * 100 + seasonNumber; + var season = new Season { Id = seasonId, ShowId = showId, SeasonNumber = seasonNumber }; + + return Enumerable.Range(0, episodes) + .Map(i => (MediaItem)new Episode + { + Id = firstId + i, + Season = season, + SeasonId = seasonId, + EpisodeMetadata = [new EpisodeMetadata { EpisodeNumber = i + 1 }], + MediaVersions = + [ + new MediaVersion + { + Duration = TimeSpan.FromMinutes(30), + MediaFiles = [new MediaFile { Path = $"/fake/path/{firstId + i}" }], + Chapters = [] + } + ] + }) + .ToList(); + } +} diff --git a/ErsatzTV.Core.Tests/Scheduling/PlaylistEnumeratorTests.cs b/ErsatzTV.Core.Tests/Scheduling/PlaylistEnumeratorTests.cs index c12593b1b..2a75ba8d7 100644 --- a/ErsatzTV.Core.Tests/Scheduling/PlaylistEnumeratorTests.cs +++ b/ErsatzTV.Core.Tests/Scheduling/PlaylistEnumeratorTests.cs @@ -457,6 +457,77 @@ public class PlaylistEnumeratorTests continued.ShouldBe(reference.Skip(6).Take(6)); } + /// + /// Completing a cycle reshuffles the playlist items, and the state persisted at that point is only a + /// seed and an index. Rebuilding starts from the playlist order and applies the seed, so the shuffle + /// has to be a function of those two things - otherwise the rebuilt playout diverges from the one + /// the previous build was part way through. + /// + [Test] + public async Task Shuffled_Playlist_Should_Resume_The_Same_Way_After_A_Cycle() + { + IMediaCollectionRepository repo = Substitute.For(); + + // four single-item entries, so a cycle is four moves and each move names its own playlist item + Dictionary> BuildMap() => + Range(1, 4).ToDictionary( + i => new PlaylistItem + { + Id = i, + Index = i - 1, + PlaybackOrder = PlaybackOrder.Chronological, + PlayAll = false, + CollectionType = CollectionType.Collection, + CollectionId = i + }, + i => new List { FakeMovie(i) }); + + const int SEED = 12345; + + PlaylistEnumerator live = await PlaylistEnumerator.Create( + repo, + BuildMap(), + new CollectionEnumeratorState { Seed = SEED, Index = 0 }, + shufflePlaylistItems: true, + batchSize: Option.None, + randomStartPoint: false, + CancellationToken.None); + + // play past the end of the first cycle, which reseeds and reshuffles + for (var i = 0; i < 6; i++) + { + live.MoveNext(Option.None); + } + + CollectionEnumeratorState persisted = live.State.Clone(); + persisted.Seed.ShouldNotBe(SEED, "the cycle should have completed and reseeded"); + + var expected = new List(); + for (var i = 0; i < 4; i++) + { + expected.AddRange(live.Current.Map(mi => mi.Id)); + live.MoveNext(Option.None); + } + + PlaylistEnumerator rebuilt = await PlaylistEnumerator.Create( + repo, + BuildMap(), + persisted.Clone(), + shufflePlaylistItems: true, + batchSize: Option.None, + randomStartPoint: false, + CancellationToken.None); + + var continued = new List(); + for (var i = 0; i < 4; i++) + { + continued.AddRange(rebuilt.Current.Map(mi => mi.Id)); + rebuilt.MoveNext(Option.None); + } + + continued.ShouldBe(expected); + } + private static Movie FakeMovie(int id) => new() { Id = id, diff --git a/ErsatzTV.Core/Scheduling/PlaylistEnumerator.cs b/ErsatzTV.Core/Scheduling/PlaylistEnumerator.cs index 21212cbb0..87c97e7fb 100644 --- a/ErsatzTV.Core/Scheduling/PlaylistEnumerator.cs +++ b/ErsatzTV.Core/Scheduling/PlaylistEnumerator.cs @@ -14,6 +14,12 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator private CloneableRandom _random; private bool _shufflePlaylistItems; private List _sortedEnumerators; + + // a cycle start is the only position this enumerator can be put back to directly; every other + // position is reached by replaying from one. see RewindToCycleStart + private List _enumeratorsInPlaylistOrder; + private Dictionary _childStatesAtCycleStart; + private int _itemsTakenFromCurrent; private Option _batchSize = Option.None; @@ -31,10 +37,20 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator public int EnumeratorIndex { get; private set; } + /// + /// A playlist position isn't described by index and seed alone - it also lives in + /// , the batch progress, the ids the cycle still owes and the state of + /// every child enumerator - so the only way back to a position is to replay to it. + /// public void ResetState(CollectionEnumeratorState state) { - // seed doesn't matter here - State.Index = state.Index; + // nothing has moved since this state was handed out, so there is nothing to rewind + if (State.Index != state.Index || State.Seed != state.Seed) + { + RewindToCycleStart(state.Seed); + ReplayTo(state.Index); + } + State.Started = state.Started; } @@ -147,14 +163,12 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator foreach (PlaylistItem playlistItem in playlistItemMap.Keys.OrderBy(i => i.Index)) { List items = playlistItemMap[playlistItem]; - foreach (MediaItem mediaItem in items) - { - result._allMediaItemIds.Add(mediaItem.Id); - if (playlistItem.IncludeInProgramGuide) - { - result._idsToIncludeInEPG.Add(mediaItem.Id); - } + // a cycle ends when every item has been played, so an item that can never be played would + // leave the cycle permanently incomplete; season, episode order won't play season 0 + if (playlistItem.PlaybackOrder is PlaybackOrder.SeasonEpisode) + { + items = SeasonEpisodeMediaCollectionEnumerator.Playable(items); } var collectionKey = CollectionKey.ForPlaylistItem(playlistItem); @@ -162,6 +176,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator { result._sortedEnumerators.Add( new EnumeratorPlayAllCount(enumerator, playlistItem.PlayAll, playlistItem.Count)); + result.TrackMediaItemIds(items, playlistItem.IncludeInProgramGuide); continue; } @@ -211,7 +226,8 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator } enumerator = new SeasonEpisodeMediaCollectionEnumerator(items, initState); - // season, episode will filter out season 0, so we may get an empty enumerator back + // every season 0 episode was already removed above, so this is a playlist item + // that had nothing else in it if (enumerator.Count == 0) { enumerator = null; @@ -229,6 +245,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator enumeratorMap.Add(collectionKey, enumerator); result._sortedEnumerators.Add( new EnumeratorPlayAllCount(enumerator, playlistItem.PlayAll, playlistItem.Count)); + result.TrackMediaItemIds(items, playlistItem.IncludeInProgramGuide); } } @@ -240,6 +257,13 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator .OrderBy(identity) .HeadOrNone(); + // captured before anything moves, so RewindToCycleStart can put the children back + result._enumeratorsInPlaylistOrder = [.. result._sortedEnumerators]; + result._childStatesAtCycleStart = result._sortedEnumerators + .Map(e => e.Enumerator) + .Distinct() + .ToDictionary(e => e, e => e.State.Clone()); + result._random = new CloneableRandom(state.Seed); if (shufflePlaylistItems) @@ -256,16 +280,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator state.Index = 0; } - while (result.State.Index < state.Index) - { - result.MoveNext(Option.None); - - // previous state is no longer valid; playlist now has fewer items - if (result.State.Index == 0) - { - break; - } - } + result.ReplayTo(state.Index); var childEnumerators = new List(); foreach ((IMediaCollectionEnumerator enumerator, _, _) in result._sortedEnumerators) @@ -281,15 +296,82 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator return result; } + private void TrackMediaItemIds(List items, bool includeInProgramGuide) + { + foreach (MediaItem mediaItem in items) + { + _allMediaItemIds.Add(mediaItem.Id); + + if (includeInProgramGuide) + { + _idsToIncludeInEPG.Add(mediaItem.Id); + } + } + } + + /// + /// Puts everything back the way leaves it for the given seed: children at their + /// starting positions, the playlist items back in seed order, and the cycle owing every item again. + /// + private void RewindToCycleStart(int seed) + { + foreach ((IMediaCollectionEnumerator enumerator, CollectionEnumeratorState childState) in + _childStatesAtCycleStart) + { + enumerator.ResetState(childState.Clone()); + } + + _sortedEnumerators = [.. _enumeratorsInPlaylistOrder]; + _random = new CloneableRandom(seed); + + if (_shufflePlaylistItems) + { + _sortedEnumerators = ShufflePlaylistItems(); + } + + _remainingMediaItemIds.Clear(); + _remainingMediaItemIds.UnionWith(_allMediaItemIds); + + EnumeratorIndex = 0; + _itemsTakenFromCurrent = 0; + + State.Seed = seed; + State.Index = 0; + } + + private void ReplayTo(int index) + { + if (_sortedEnumerators.Count == 0) + { + return; + } + + while (State.Index < index) + { + MoveNext(Option.None); + + // previous state is no longer valid; playlist now has fewer items + if (State.Index == 0) + { + break; + } + } + } + + /// + /// Always shuffles the playlist order rather than the current order, so that the result is a function + /// of that order and the seed - which is all the persisted state gives a rebuild to work from. Cycles + /// reshuffle from the same starting point rather than from wherever the last shuffle landed. + /// private List ShufflePlaylistItems() { - if (_sortedEnumerators.Count < 3) + if (_enumeratorsInPlaylistOrder.Count < 3) { - return _sortedEnumerators; + return [.. _enumeratorsInPlaylistOrder]; } - EnumeratorPlayAllCount[] copy = _sortedEnumerators.ToArray(); - EnumeratorPlayAllCount last = _sortedEnumerators.Last(); + EnumeratorPlayAllCount[] copy = _enumeratorsInPlaylistOrder.ToArray(); + EnumeratorPlayAllCount last = _enumeratorsInPlaylistOrder.Last(); do { diff --git a/ErsatzTV.Core/Scheduling/SeasonEpisodeMediaCollectionEnumerator.cs b/ErsatzTV.Core/Scheduling/SeasonEpisodeMediaCollectionEnumerator.cs index 10845e45a..cff1577fb 100644 --- a/ErsatzTV.Core/Scheduling/SeasonEpisodeMediaCollectionEnumerator.cs +++ b/ErsatzTV.Core/Scheduling/SeasonEpisodeMediaCollectionEnumerator.cs @@ -15,8 +15,7 @@ public sealed class SeasonEpisodeMediaCollectionEnumerator : IMediaCollectionEnu { CurrentIncludeInProgramGuide = Option.None; - _sortedMediaItems = mediaItems - .Filter(mi => (mi is not Episode episode) || (episode.Season?.SeasonNumber ?? 0) > 0) + _sortedMediaItems = Playable(mediaItems) .OrderBy(identity, new SeasonEpisodeMediaComparer()).ToList(); _lazyMinimumDuration = new Lazy>(() => _sortedMediaItems.Bind(i => i.GetNonZeroDuration()).OrderBy(identity).HeadOrNone()); @@ -35,6 +34,13 @@ public sealed class SeasonEpisodeMediaCollectionEnumerator : IMediaCollectionEnu } } + /// + /// Season 0 is never played in season, episode order. Callers that need to know which items this + /// enumerator will actually play - rather than which items they handed it - share the rule here. + /// + public static List Playable(IEnumerable mediaItems) => + mediaItems.Filter(mi => (mi is not Episode episode) || (episode.Season?.SeasonNumber ?? 0) > 0).ToList(); + public void ResetState(CollectionEnumeratorState state) { // seed doesn't matter here