[core] Allow the StreamingPicker to pick any piece

A prior change to ensure pieces from within the high, or low
priority range were always prioritised ahead of pieces from
the 'not important at all' range accidentally made the picker
not chooose pieces from the 'not important at all' set.

This would have little significance for real world streaming
scenarios as the 'not important at all' set are pieces which
come are before the current file position, i.e. pieces the
media player would not try to use.

Fixes #425

Author: alanmcgovern

src/MonoTorrent.Tests/Client/StreamingPieceRequesterTests.cs

@@ -0,0 +1,109 @@
+//
+// StreamingPieceRequesterTests.cs
+//
+// Authors:
+//   Alan McGovern [email protected]
+//
+// Copyright (C) 2021 Alan McGovern
+//
+// Permission is hereby granted, free of charge, to any person obtaining
+// a copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to
+// permit persons to whom the Software is furnished to do so, subject to
+// the following conditions:
+// 
+// The above copyright notice and this permission notice shall be
+// included in all copies or substantial portions of the Software.
+// 
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+//
+
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+
+using MonoTorrent.Client.Messages;
+using MonoTorrent.Client.Messages.Standard;
+
+using NUnit.Framework;
+
+namespace MonoTorrent.Client.PiecePicking
+{
+    [TestFixture]
+    public class StreamingPieceRequesterTests
+    {
+        class TorrentData : ITorrentData
+        {
+            public IList<ITorrentFileInfo> Files { get; } = TorrentFileInfo.Create (Piece.BlockSize * 8, 1024 * 1024 * 8);
+            public int PieceLength  => Piece.BlockSize * 8;
+            public long Size => Files[0].Length;
+        }
+
+        [Test]
+        public void PickFromBeforeHighPrioritySet ()
+        {
+            var data = new TorrentData ();
+            var ignoringBitfield = new MutableBitField (data.PieceCount ())
+                .SetAll (true)
+                .Set (0, false);
+
+            var requester = new StreamingPieceRequester ();
+            requester.Initialise (data, new[] { ignoringBitfield });
+            requester.SeekToPosition (data.Files[0], data.PieceLength * 3);
+
+            var peer = PeerId.CreateNull (ignoringBitfield.Length, true, false, true);
+            requester.AddRequests (peer, Array.Empty<IPeerWithMessaging> ());
+            Assert.AreEqual (2, peer.AmRequestingPiecesCount);
+
+            var requests = GetRequests (peer);
+            Assert.AreEqual (2, requests.Count);
+            Assert.IsTrue (requests.All (r => r.PieceIndex == 0));
+        }
+
+        [Test]
+        public void PickHighestPriority ()
+        {
+            var data = new TorrentData ();
+            var ignoringBitfield = new MutableBitField (data.PieceCount ())
+                .SetAll (false);
+
+            var requester = new StreamingPieceRequester ();
+            requester.Initialise (data, new[] { ignoringBitfield });
+            requester.SeekToPosition (data.Files[0], data.PieceLength * 3);
+
+            var peer = PeerId.CreateNull (ignoringBitfield.Length, true, false, true);
+            requester.AddRequests (peer, Array.Empty<IPeerWithMessaging> ());
+            Assert.AreEqual (4, peer.AmRequestingPiecesCount);
+
+            var requests = GetRequests (peer);
+            Assert.AreEqual (4, requests.Count);
+            Assert.IsTrue (requests.All (r => r.PieceIndex == 3));
+        }
+
+        static List<RequestMessage> GetRequests (PeerId peer)
+        {
+            List<RequestMessage> results = new List<RequestMessage> ();
+            while (peer.MessageQueue.QueueLength > 0) {
+                var message = peer.MessageQueue.TryDequeue ();
+                if (message is RequestMessage r) {
+                    results.Add (r);
+                } else if (message is RequestBundle bundle) {
+                    foreach (var inner in bundle.ToRequestMessages ())
+                        if (inner is RequestMessage req)
+                            results.Add (req);
+                }
+            }
+            return results;
+        }
+
+    }
+}

src/MonoTorrent/MonoTorrent.Client.PiecePicking/StreamingPieceRequester.cs

@@ -121,7 +121,7 @@ public void AddRequests (IReadOnlyList<IPeerWithMessaging> peers)
             foreach (var supportsFastPeer in new[] { true, false }) {
                 for (int i = 0; i < 4; i++) {
                     foreach (var peer in fastestPeers.Where (p => p.SupportsFastPeer == supportsFastPeer)) {
-                        AddRequests (peer, peers, HighPriorityPieceIndex, Math.Min (HighPriorityPieceIndex + 1, bitfield.Length - 1), 2, (i + 1) * 2);
+                        AddRequests (peer, peers, HighPriorityPieceIndex, Math.Min (HighPriorityPieceIndex + 1, bitfield.Length - 1), 2, preferredMaxRequests : (i + 1) * 2);
                     }
                 }
             }
@@ -136,19 +136,20 @@ public void AddRequests (IPeerWithMessaging peer, IReadOnlyList<IPeerWithMessagi
             // The first two pieces in the high priority set should be requested multiple times to ensure fast delivery
             var pieceCount = TorrentData.PieceCount ();
             for (int i = HighPriorityPieceIndex; i < pieceCount && i <= HighPriorityPieceIndex + 1; i++)
-                AddRequests (peer, allPeers, HighPriorityPieceIndex, HighPriorityPieceIndex, 2);
+                AddRequests (peer, allPeers, HighPriorityPieceIndex, HighPriorityPieceIndex, 2, preferredMaxRequests: 4);
 
             var lowPriorityEnd = Math.Min (pieceCount - 1, HighPriorityPieceIndex + LowPriorityCount - 1);
-            AddRequests (peer, allPeers, HighPriorityPieceIndex, lowPriorityEnd, 1);
+            AddRequests (peer, allPeers, HighPriorityPieceIndex, lowPriorityEnd, 1, preferredMaxRequests: 3);
+            AddRequests (peer, allPeers, 0, pieceCount - 1, 1, preferredMaxRequests: 2);
         }
 
-        void AddRequests (IPeerWithMessaging peer, IReadOnlyList<IPeerWithMessaging> allPeers, int startPieceIndex, int endPieceIndex, int maxDuplicates, int? preferredMaxRequests = null)
+        void AddRequests (IPeerWithMessaging peer, IReadOnlyList<IPeerWithMessaging> allPeers, int startPieceIndex, int endPieceIndex, int maxDuplicates, int preferredMaxRequests)
         {
             if (!peer.CanRequestMorePieces)
                 return;
 
             int preferredRequestAmount = peer.PreferredRequestAmount (TorrentData.PieceLength);
-            var maxRequests = Math.Min (preferredMaxRequests ?? 3, peer.MaxPendingRequests);
+            var maxRequests = Math.Min (preferredMaxRequests, peer.MaxPendingRequests);
 
             if (peer.AmRequestingPiecesCount >= maxRequests)
                 return;
@@ -185,23 +186,13 @@ void AddRequests (IPeerWithMessaging peer, IReadOnlyList<IPeerWithMessaging> all
                 MutableBitField filtered = null;
                 while (peer.AmRequestingPiecesCount < maxRequests) {
                     filtered ??= GenerateAlreadyHaves ().Not ().And (peer.BitField);
-                    IList<BlockInfo> request = PriorityPick (peer, filtered, allPeers, preferredRequestAmount, 0, TorrentData.PieceCount () - 1);
+                    IList<BlockInfo> request = PriorityPick (peer, filtered, allPeers, preferredRequestAmount, startPieceIndex, endPieceIndex);
                     if (request != null && request.Count > 0)
                         peer.EnqueueRequests (request);
                     else
                         break;
                 }
             }
-
-            if (!peer.IsChoking && peer.AmRequestingPiecesCount == 0) {
-                while (peer.AmRequestingPiecesCount < maxRequests) {
-                    BlockInfo? request = Picker.ContinueAnyExistingRequest (peer, HighPriorityPieceIndex, TorrentData.PieceCount () - 1, 1);
-                    if (request != null)
-                        peer.EnqueueRequest (request.Value);
-                    else
-                        break;
-                }
-            }
         }
 
         MutableBitField GenerateAlreadyHaves ()
@@ -234,18 +225,15 @@ IList<BlockInfo> PriorityPick (IPeer peer, BitField available, IReadOnlyList<IPe
                     return bundle;
             }
 
-            if (endIndex < HighPriorityPieceIndex)
-                return null;
-
             var lowPriorityEndIndex = Math.Min (HighPriorityPieceIndex + LowPriorityCount, endIndex);
             if ((bundle = LowPriorityPicker.PickPiece (peer, available, otherPeers, count, HighPriorityPieceIndex, lowPriorityEndIndex)) != null)
                 return bundle;
 
             // If we're downloading from the 'not important at all' section, queue up at most 2.
-            if (peer.AmRequestingPiecesCount > 2)
-                return null;
+            if (peer.AmRequestingPiecesCount < 2)
+                return LowPriorityPicker.PickPiece (peer, available, otherPeers, count, startIndex, endIndex);
 
-            return LowPriorityPicker.PickPiece (peer, available, otherPeers, count, HighPriorityPieceIndex, endIndex);
+            return null;
         }
 
         /// <summary>