Repository: postlund/pyatv (python)

Issue:
Support retransmission of lost packets when streaming
**Short feature/function description**
An AirPlay receiver can request old packets to be retransmitted in case they are missing. According to [here](https://git.zx2c4.com/Airtunes2/about/#constants), 1000 packets shall be buffered. FIFO is assumed. Retransmission requests are made on the control channel.

**What needs to be done?**

* Create a FIFO (ring buffer) for all packets and attach sequence number
* Handle retransmission requests on the control channel

**Is this a breaking change?**
No

**Anything else worth knowing?**
Assuming two channels, sample size of 16bit and 352 frames per packet, this will consume a bit of memory: 2 * 2 * 352 * 1000=1,4MB

Pull request:
raop: Support retransmission of lost packets
Buffer the last 1000 packets and re-send if requested by receiver.

Fixes #1079

<a href="https://gitpod.io/#https://github.com/postlund/pyatv/pull/1099"><img src="https://gitpod.io/button/open-in-gitpod.svg"/></a>



Patch:
diff --git a/docs/documentation/supported_features.md b/docs/documentation/supported_features.md
index a9203a47..102c27ef 100644
--- a/docs/documentation/supported_features.md
+++ b/docs/documentation/supported_features.md
@@ -196,7 +196,6 @@ AirTunes).
 * Devices requiring HAP pairing, e.g. Apple TV 4+, are not supported {% include issue no="1078" %}
 * Track length is hardcoded to 3s for now
 * Remote control commands does not work, e.g. play or pause {% include issue no="1068" %}
-* Retransmission of lost packets is not supported {% include issue no="1079" %}
 
 ### Verified Devices
 
diff --git a/pyatv/raop/fifo.py b/pyatv/raop/fifo.py
new file mode 100644
index 00000000..714d5d6b
--- /dev/null
+++ b/pyatv/raop/fifo.py
@@ -0,0 +1,81 @@
+"""Simple FIFO for packets based on dict.
+
+This FIFO holds a certain number of elements as defined by upper_limit. Each item maps a
+sequence number to a packet, allowing fast look up of a certain packet. The order is
+defined by insertion order and *not* sequence number order.
+
+When upper limit is exceeded, the item that was *inserted* last, is removed.
+
+Example:
+fifo = PacketFifo(2)
+fifo[1] = 123
+fifo[2] = 456
+print(fifo[1], fifo[2])
+"""
+
+from typing import Dict, Iterator, MutableMapping, TypeVar
+
+T = TypeVar("T")
+
+
+class PacketFifo(MutableMapping[int, T]):  # pylint: disable=too-many-ancestors
+    """Implementation of simple packet FIFO."""
+
+    _items: Dict[int, T]
+
+    def __init__(self, upper_limit: int) -> None:
+        """Initialize a new PacketFifo instance."""
+        self._items = {}
+        self._upper_limit = upper_limit
+
+    def clear(self):
+        """Remove all items in FIFO."""
+        self._items.clear()
+
+    def __len__(self) -> int:
+        """Return number of items in FIFO."""
+        return len(self._items)
+
+    def __setitem__(self, index: int, value: T):
+        """Add an items to FIFO."""
+        if isinstance(index, int):
+            # Cannot add item with same index again
+            if index in self._items:
+                raise ValueError(f"{index} already in FIFO")
+
+            # Remove oldest item if limit is exceeded
+            if len(self) + 1 > self._upper_limit:
+                del self._items[list(self._items.keys())[0]]
+
+            self._items[index] = value
+        else:
+            raise TypeError("only int supported as key")
+
+    def __delitem__(self, index: int) -> None:
+        """Remove item from FIFO."""
+        raise NotImplementedError("removing items not supported")
+
+    def __iter__(self) -> Iterator[int]:
+        """Iterate over indices in FIFO."""
+        return self._items.__iter__()
+
+    def __getitem__(self, index: int) -> T:
+        """Return value of an item."""
+        if isinstance(index, int):
+            return self._items[index]
+        raise TypeError("only int supported as key")
+
+    def __contains__(self, index: object) -> bool:
+        """Return if an element exists in FIFO."""
+        return index in self._items
+
+    def __str__(self) -> str:
+        """Return string representation of FIFO.
+
+        Only index numbers are returned in the string.
+        """
+        return str(list(self._items.keys()))
+
+    def __repr__(self) -> str:
+        """Return internal representation as string of FIFO."""
+        return repr(self._items)
diff --git a/pyatv/raop/packets.py b/pyatv/raop/packets.py
index 8706b633..9b977bd0 100644
--- a/pyatv/raop/packets.py
+++ b/pyatv/raop/packets.py
@@ -55,3 +55,7 @@ AudioPacketHeader = RtpHeader.extend(
     timestamp="I",
     ssrc="I",
 )
+
+RetransmitReqeust = RtpHeader.extend(
+    "RetransmitPacket", lost_seqno="H", lost_packets="H"
+)
diff --git a/pyatv/raop/raop.py b/pyatv/raop/raop.py
index 5b00d0e2..a58a1d80 100644
--- a/pyatv/raop/raop.py
+++ b/pyatv/raop/raop.py
@@ -11,8 +11,14 @@ from bitarray import bitarray
 
 from pyatv import exceptions
 from pyatv.raop import timing
+from pyatv.raop.fifo import PacketFifo
 from pyatv.raop.metadata import EMPTY_METADATA, AudioMetadata
-from pyatv.raop.packets import AudioPacketHeader, SyncPacket, TimingPacket
+from pyatv.raop.packets import (
+    AudioPacketHeader,
+    RetransmitReqeust,
+    SyncPacket,
+    TimingPacket,
+)
 from pyatv.raop.parsers import (
     EncryptionType,
     MetadataType,
@@ -28,6 +34,9 @@ _LOGGER = logging.getLogger(__name__)
 # When being late, compensate by sending at most these many packets to catch up
 MAX_PACKETS_COMPENSATE = 3
 
+# We should store this many packets in case retransmission is requested
+PACKET_BACKLOG_SIZE = 1000
+
 KEEP_ALIVE_INTERVAL = 25  # Seconds
 
 # Metadata used when no metadata is present
@@ -41,10 +50,11 @@ SUPPORTED_ENCRYPTIONS = EncryptionType.Unencrypted | EncryptionType.MFiSAP
 class ControlClient(asyncio.Protocol):
     """Control client responsible for e.g. sync packets."""
 
-    def __init__(self, context: RtspContext):
+    def __init__(self, context: RtspContext, packet_backlog: PacketFifo):
         """Initialize a new ControlClient."""
         self.transport = None
         self.context = context
+        self.packet_backlog = packet_backlog
         self.task: Optional[asyncio.Future] = None
 
     def close(self):
@@ -118,10 +128,31 @@ class ControlClient(asyncio.Protocol):
         """Handle that connection succeeded."""
         self.transport = transport
 
-    @staticmethod
-    def datagram_received(data, addr):
+    def datagram_received(self, data, addr):
         """Handle incoming control data."""
-        _LOGGER.debug("Received control data from %s: %s", addr, data)
+        actual_type = data[1] & 0x7F  # Remove marker bit
+
+        if actual_type == 0x55:
+            self._retransmit_lost_packets(RetransmitReqeust.decode(data), addr)
+        else:
+            _LOGGER.debug("Received unhandled control data from %s: %s", addr, data)
+
+    def _retransmit_lost_packets(self, request: RetransmitReqeust, addr):
+        _LOGGER.debug("%s from %s", request, addr)
+
+        for i in range(request.lost_packets):
+            if request.lost_seqno + i in self.packet_backlog:
+                packet = self.packet_backlog[request.lost_seqno + i]
+
+                # Very "low level" here just because it's simple and avoids
+                # unnecessary conversions
+                original_seqno = packet[2:4]
+                resp = b"\x80\xD6" + original_seqno + packet
+
+                if self.transport:
+                    self.transport.sendto(resp, addr)
+            else:
+                _LOGGER.debug("Packet %d not in backlog", request.lost_seqno + 1)
 
     @staticmethod
     def error_received(exc):
@@ -244,6 +275,7 @@ class RaopClient:
         self.context: RtspContext = context
         self.control_client: Optional[ControlClient] = None
         self.timing_client: Optional[TimingClient] = None
+        self._packet_backlog: PacketFifo = PacketFifo(PACKET_BACKLOG_SIZE)
         self._encryption_types: EncryptionType = EncryptionType.Unknown
         self._metadata_types: MetadataType = MetadataType.NotSupported
         self._metadata: AudioMetadata = EMPTY_METADATA
@@ -316,7 +348,8 @@ class RaopClient:
         self._update_output_properties(properties)
 
         (_, control_client) = await self.loop.create_datagram_endpoint(
-            lambda: ControlClient(self.context), local_addr=(self.rtsp.local_ip, 0)
+            lambda: ControlClient(self.context, self._packet_backlog),
+            local_addr=(self.rtsp.local_ip, 0),
         )
         (_, timing_client) = await self.loop.create_datagram_endpoint(
             TimingClient, local_addr=(self.rtsp.local_ip, 0)
@@ -422,6 +455,7 @@ class RaopClient:
         except Exception as ex:
             raise exceptions.ProtocolError("an error occurred during streaming") from ex
         finally:
+            self._packet_backlog.clear()  # Don't keep old packets around (big!)
             if transport:
                 transport.close()
             if self._keep_alive_task:
@@ -518,16 +552,21 @@ class RaopClient:
         for i in range(0, len(frames), 2):
             audio.frombytes(bytes([frames[i + 1], frames[i]]))
 
+        if transport.is_closing():
+            _LOGGER.warning("Connection closed while streaming audio")
+            return 0
+
+        packet = header + audio.tobytes()
+
+        # Add packet to backlog before sending
+        self._packet_backlog[self.context.rtpseq] = packet
+        transport.sendto(packet)
+
         self.context.rtpseq = (self.context.rtpseq + 1) % (2 ** 16)
         self.context.head_ts += int(
             len(frames) / (self.context.channels * self.context.bytes_per_channel)
         )
 
-        if transport.is_closing():
-            _LOGGER.warning("Connection closed while streaming audio")
-            return 0
-
-        transport.sendto(header + audio.tobytes())
         return int(
             len(frames) / (self.context.channels * self.context.bytes_per_channel)
         )


Test patch:
diff --git a/tests/raop/test_fifo.py b/tests/raop/test_fifo.py
new file mode 100644
index 00000000..c0d6f072
--- /dev/null
+++ b/tests/raop/test_fifo.py
@@ -0,0 +1,126 @@
+"""Unit tests for pyatv.raop.fifo."""
+
+import pytest
+
+from pyatv.raop.fifo import PacketFifo
+
+
+def test_add_to_fifo():
+    fifo = PacketFifo(10)
+    assert not fifo
+
+    fifo[123] = 456
+    assert len(fifo) == 1
+    assert fifo
+
+
+def test_add_existing_to_fifo():
+    fifo = PacketFifo(10)
+    fifo[123] = 456
+
+    with pytest.raises(ValueError):
+        fifo[123] = 789
+
+
+def test_add_index_not_int_raises():
+    fifo = PacketFifo(10)
+
+    with pytest.raises(TypeError):
+        fifo["test"] = 123
+
+
+def test_get_missing_from_fifo():
+    fifo = PacketFifo(10)
+
+    with pytest.raises(KeyError):
+        fifo[123]
+
+
+def test_get_index_not_int_raises():
+    fifo = PacketFifo(10)
+
+    with pytest.raises(TypeError):
+        fifo["test"]
+
+
+def test_in_operator():
+    fifo = PacketFifo(10)
+    assert 1 not in fifo
+
+    fifo[1] = 0
+    assert 1 in fifo
+
+
+def test_add_multiple():
+    fifo = PacketFifo(10)
+    fifo[0] = 1
+    fifo[1] = 2
+    fifo[2] = 3
+    assert fifo[0] == 1
+    assert fifo[1] == 2
+    assert fifo[2] == 3
+    assert len(fifo) == 3
+    assert fifo
+
+
+def test_add_overflow_removes_oldest():
+    fifo = PacketFifo(2)
+    fifo[0] = 1
+    fifo[1] = 2
+
+    fifo[2] = 3
+    assert len(fifo) == 2
+    assert 0 not in fifo
+    assert fifo[1] == 2
+    assert fifo[2] == 3
+
+    fifo[3] = 4
+    assert len(fifo) == 2
+    assert 1 not in fifo
+    assert fifo[2] == 3
+    assert fifo[3] == 4
+
+
+def test_clear_fifo():
+    fifo = PacketFifo(2)
+    fifo[0] = 1
+    fifo[1] = 2
+    fifo.clear()
+    assert len(fifo) == 0
+
+
+def test_del_not_supported():
+    fifo = PacketFifo(2)
+    fifo[0] = 1
+
+    with pytest.raises(NotImplementedError):
+        del fifo[0]
+
+
+def test_iter_over_indices():
+    fifo = PacketFifo(10)
+    fifo[1] = 1
+    fifo[2] = 2
+    fifo[3] = 2
+
+    sum = 0
+    for i in fifo:
+        sum += i
+
+    assert sum == (1 + 2 + 3)
+
+
+def test_str():
+    fifo = PacketFifo(2)
+    assert str(fifo) == "[]"
+    fifo[1] = 2
+    fifo[2] = 3
+    assert str(fifo) == "[1, 2]"
+
+
+def test_repr():
+    fifo = PacketFifo(2)
+    assert repr(fifo) == "{}"
+    fifo[1] = 2
+    fifo[2] = 3
+    assert repr(fifo) == "{1: 2, 2: 3}"