fix: playlist enumerator consistency fixes

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
 

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;
+
+/// <summary>
+///     Marathon content builds a <see cref="PlaylistEnumerator" /> over one playlist item per group,
+///     so it inherits that enumerator's save/restore behaviour. These cover the state handling, not
+///     the grouping.
+/// </summary>
+[TestFixture]
+public class MarathonEnumeratorTests
+{
+    private const int Seed = 987654321;
+
+    [SetUp]
+    public void SetUp() => _cancellationToken = new CancellationTokenSource(TimeSpan.FromSeconds(10)).Token;
+
+    private CancellationToken _cancellationToken;
+
+    /// <summary>
+    ///     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.
+    /// </summary>
+    [Test]
+    public async Task Marathon_Should_Complete_A_Cycle()
+    {
+        List<MediaItem> 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<int> indexes = Advance(enumerator, 12);
+
+        indexes.ShouldContain(0, "the index never returned to 0, so the cycle never completed");
+    }
+
+    /// <summary>
+    ///     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.
+    /// </summary>
+    [Test]
+    public async Task Marathon_Should_Complete_A_Cycle_With_Specials()
+    {
+        List<MediaItem> 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<int> indexes = Advance(enumerator, 60);
+
+        indexes.ShouldContain(0, "the index never returned to 0, so the cycle never completed");
+    }
+
+    /// <summary>
+    ///     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.
+    /// </summary>
+    [Test]
+    public async Task Marathon_Should_Rewind_On_ResetState()
+    {
+        List<MediaItem> 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<DateTimeOffset>.None);
+
+        CollectionEnumeratorState snapshot = enumerator.State.Clone();
+        int expected = enumerator.Current.Map(mi => mi.Id).IfNone(-1);
+
+        enumerator.MoveNext(Option<DateTimeOffset>.None);
+        enumerator.MoveNext(Option<DateTimeOffset>.None);
+
+        enumerator.ResetState(snapshot);
+
+        enumerator.Current.Map(mi => mi.Id).IfNone(-1).ShouldBe(expected);
+    }
+
+    /// <summary>
+    ///     Rebuilding a playout restores the enumerator from the persisted state, which has to land on
+    ///     the same item the previous build stopped at.
+    /// </summary>
+    [Test]
+    public async Task Marathon_Should_Resume_Where_It_Left_Off()
+    {
+        List<MediaItem> 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<int>();
+        var states = new List<CollectionEnumeratorState>();
+        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<DateTimeOffset>.None);
+        }
+
+        var mismatches = new List<string>();
+        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<PlaylistEnumerator> CreateMarathon(
+        List<MediaItem> mediaItems,
+        CollectionEnumeratorState state = null)
+    {
+        IMediaCollectionRepository repo = Substitute.For<IMediaCollectionRepository>();
+        var helper = new MarathonHelper(repo);
+
+        Option<PlaylistEnumerator> maybeEnumerator = await helper.GetEnumerator(
+            mediaItems,
+            MarathonGroupBy.Show,
+            marathonShuffleGroups: false,
+            marathonShuffleItems: false,
+            marathonBatchSize: Option<int>.None,
+            state ?? new CollectionEnumeratorState { Seed = Seed, Index = 0 },
+            _cancellationToken);
+
+        return maybeEnumerator.IfNone(() => throw new InvalidOperationException("no marathon enumerator"));
+    }
+
+    private static List<int> Advance(PlaylistEnumerator enumerator, int count)
+    {
+        var indexes = new List<int>();
+        for (var i = 0; i < count; i++)
+        {
+            enumerator.MoveNext(Option<DateTimeOffset>.None);
+            indexes.Add(enumerator.State.Index);
+        }
+
+        return indexes;
+    }
+
+    private static List<MediaItem> 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();
+    }
+}

ErsatzTV.Core.Tests/Scheduling/PlaylistEnumeratorTests.cs

@@ -457,6 +457,77 @@ public class PlaylistEnumeratorTests
         continued.ShouldBe(reference.Skip(6).Take(6));
     }
 
+    /// <summary>
+    ///     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.
+    /// </summary>
+    [Test]
+    public async Task Shuffled_Playlist_Should_Resume_The_Same_Way_After_A_Cycle()
+    {
+        IMediaCollectionRepository repo = Substitute.For<IMediaCollectionRepository>();
+
+        // four single-item entries, so a cycle is four moves and each move names its own playlist item
+        Dictionary<PlaylistItem, List<MediaItem>> 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<MediaItem> { FakeMovie(i) });
+
+        const int SEED = 12345;
+
+        PlaylistEnumerator live = await PlaylistEnumerator.Create(
+            repo,
+            BuildMap(),
+            new CollectionEnumeratorState { Seed = SEED, Index = 0 },
+            shufflePlaylistItems: true,
+            batchSize: Option<int>.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<DateTimeOffset>.None);
+        }
+
+        CollectionEnumeratorState persisted = live.State.Clone();
+        persisted.Seed.ShouldNotBe(SEED, "the cycle should have completed and reseeded");
+
+        var expected = new List<int>();
+        for (var i = 0; i < 4; i++)
+        {
+            expected.AddRange(live.Current.Map(mi => mi.Id));
+            live.MoveNext(Option<DateTimeOffset>.None);
+        }
+
+        PlaylistEnumerator rebuilt = await PlaylistEnumerator.Create(
+            repo,
+            BuildMap(),
+            persisted.Clone(),
+            shufflePlaylistItems: true,
+            batchSize: Option<int>.None,
+            randomStartPoint: false,
+            CancellationToken.None);
+
+        var continued = new List<int>();
+        for (var i = 0; i < 4; i++)
+        {
+            continued.AddRange(rebuilt.Current.Map(mi => mi.Id));
+            rebuilt.MoveNext(Option<DateTimeOffset>.None);
+        }
+
+        continued.ShouldBe(expected);
+    }
+
     private static Movie FakeMovie(int id) => new()
     {
         Id = id,

ErsatzTV.Core/Scheduling/PlaylistEnumerator.cs

@@ -14,6 +14,12 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator
     private CloneableRandom _random;
     private bool _shufflePlaylistItems;
     private List<EnumeratorPlayAllCount> _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<EnumeratorPlayAllCount> _enumeratorsInPlaylistOrder;
+    private Dictionary<IMediaCollectionEnumerator, CollectionEnumeratorState> _childStatesAtCycleStart;
+
     private int _itemsTakenFromCurrent;
     private Option<int> _batchSize = Option<int>.None;
 
@@ -31,10 +37,20 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator
 
     public int EnumeratorIndex { get; private set; }
 
+    /// <summary>
+    ///     A playlist position isn't described by index and seed alone - it also lives in
+    ///     <see cref="EnumeratorIndex" />, 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.
+    /// </summary>
     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<MediaItem> items = playlistItemMap[playlistItem];
-            foreach (MediaItem mediaItem in items)
-            {
-                result._allMediaItemIds.Add(mediaItem.Id);
 
-                if (playlistItem.IncludeInProgramGuide)
+            // 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)
             {
-                    result._idsToIncludeInEPG.Add(mediaItem.Id);
-                }
+                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<DateTimeOffset>.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<PlaylistEnumeratorCollectionKey>();
         foreach ((IMediaCollectionEnumerator enumerator, _, _) in result._sortedEnumerators)
@@ -281,15 +296,82 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator
         return result;
     }
 
+    private void TrackMediaItemIds(List<MediaItem> items, bool includeInProgramGuide)
+    {
+        foreach (MediaItem mediaItem in items)
+        {
+            _allMediaItemIds.Add(mediaItem.Id);
+
+            if (includeInProgramGuide)
+            {
+                _idsToIncludeInEPG.Add(mediaItem.Id);
+            }
+        }
+    }
+
+    /// <summary>
+    ///     Puts everything back the way <see cref="Create" /> 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.
+    /// </summary>
+    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<DateTimeOffset>.None);
+
+            // previous state is no longer valid; playlist now has fewer items
+            if (State.Index == 0)
+            {
+                break;
+            }
+        }
+    }
+
+    /// <summary>
+    ///     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.
+    /// </summary>
     private List<EnumeratorPlayAllCount> 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
         {

ErsatzTV.Core/Scheduling/SeasonEpisodeMediaCollectionEnumerator.cs

@@ -15,8 +15,7 @@ public sealed class SeasonEpisodeMediaCollectionEnumerator : IMediaCollectionEnu
     {
         CurrentIncludeInProgramGuide = Option<bool>.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<Option<TimeSpan>>(() =>
             _sortedMediaItems.Bind(i => i.GetNonZeroDuration()).OrderBy(identity).HeadOrNone());
@@ -35,6 +34,13 @@ public sealed class SeasonEpisodeMediaCollectionEnumerator : IMediaCollectionEnu
         }
     }
 
+    /// <summary>
+    ///     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.
+    /// </summary>
+    public static List<MediaItem> Playable(IEnumerable<MediaItem> 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