review fixes, unused code removal

Author: brzep

lib/ex_webrtc/rtp/h264/depayloader.ex

@@ -1,11 +1,11 @@
 defmodule ExWebRTC.RTP.Depayloader.H264 do
-  @moduledoc """
-  Extracts H264 NAL Units from RTP packets.
+  @moduledoc false
+  # Extracts H264 NAL Units from RTP packets.
+  #
+  # Based on [RFC 6184](https://tools.ietf.org/html/rfc6184).
+  #
+  # Supported types: Single NALU, FU-A, STAP-A.
 
-  Based on [RFC 6184](https://tools.ietf.org/html/rfc6184).
-
-  Supported types: Single NALU, FU-A, STAP-A.
-  """
   @behaviour ExWebRTC.RTP.Depayloader.Behaviour
 
   require Logger
@@ -15,11 +15,11 @@ defmodule ExWebRTC.RTP.Depayloader.H264 do
   @annexb_prefix <<1::32>>
 
   @type t() :: %__MODULE__{
-          current_timestamp: nil,
-          fu_parser_acc: nil
+          current_timestamp: non_neg_integer() | nil,
+          fu_parser_acc: [binary()]
         }
 
-  defstruct [:current_timestamp, :fu_parser_acc]
+  defstruct current_timestamp: nil, fu_parser_acc: []
 
   @impl true
   def new() do
@@ -33,7 +33,7 @@ defmodule ExWebRTC.RTP.Depayloader.H264 do
     with {:ok, {header, _payload} = nal} <- NAL.Header.parse_unit_header(packet.payload),
          unit_type = NAL.Header.decode_type(header),
          {:ok, {nal, depayloader}} <-
-           handle_unit_type(unit_type, depayloader, packet, nal) do
+           do_depayload(unit_type, depayloader, packet, nal) do
       {nal, depayloader}
     else
       {:error, reason} ->
@@ -42,54 +42,60 @@ defmodule ExWebRTC.RTP.Depayloader.H264 do
         Resetting depayloader state. Payload: #{inspect(packet.payload)}.\
         """)
 
-        {nil, %{depayloader | current_timestamp: nil, fu_parser_acc: nil}}
+        {nil, %{depayloader | current_timestamp: nil, fu_parser_acc: []}}
     end
   end
 
-  defp handle_unit_type(:single_nalu, depayloader, packet, {_header, payload}) do
+  defp do_depayload(:single_nalu, depayloader, packet, {_header, payload}) do
     {:ok,
      {prefix_annexb(payload), %__MODULE__{depayloader | current_timestamp: packet.timestamp}}}
   end
 
-  defp handle_unit_type(
+  defp do_depayload(
          :fu_a,
          %{current_timestamp: current_timestamp, fu_parser_acc: fu_parser_acc},
          packet,
+         {_header, _payload}
+       )
+       when fu_parser_acc != [] and current_timestamp != packet.timestamp do
+    Logger.warning("""
+    received packet with fu-a type payload that is not a start of fragmentation unit with timestamp \
+    different than last start and without finishing the previous fu. dropping fu.\
+    """)
+
+    {:error, "invalid timestamp inside fu-a"}
+  end
+
+  defp do_depayload(
+         :fu_a,
+         %{fu_parser_acc: fu_parser_acc},
+         packet,
          {header, payload}
        ) do
-    if fu_parser_acc != nil and current_timestamp != packet.timestamp do
-      Logger.warning("""
-      Received packet with FU-A type payload that is not a start of Fragmentation Unit with timestamp \
-      different than last start and without finishing the previous FU. Dropping FU.\
-      """)
-
-      {:error, "Invalid timestamp inside FU-A"}
-    else
-      case FU.parse(payload, fu_parser_acc || %FU{}) do
-        {:ok, {data, type}} ->
-          data = NAL.Header.add_header(data, 0, header.nal_ref_idc, type)
+    case FU.parse(payload, fu_parser_acc || []) do
+      {:ok, {data, type}} ->
+        data = NAL.Header.add_header(data, 0, header.nal_ref_idc, type)
 
-          {:ok,
-           {prefix_annexb(data),
-            %__MODULE__{current_timestamp: packet.timestamp, fu_parser_acc: nil}}}
+        {:ok,
+         {prefix_annexb(data),
+          %__MODULE__{current_timestamp: packet.timestamp, fu_parser_acc: []}}}
 
-        {:incomplete, fu} ->
-          {:ok, {nil, %__MODULE__{fu_parser_acc: fu, current_timestamp: packet.timestamp}}}
+      {:incomplete, fu} ->
+        {:ok, {nil, %__MODULE__{fu_parser_acc: fu, current_timestamp: packet.timestamp}}}
 
-        {:error, _reason} = error ->
-          error
-      end
+      {:error, _reason} = error ->
+        error
     end
   end
 
-  defp handle_unit_type(:stap_a, depayloader, packet, {_header, payload}) do
+  defp do_depayload(:stap_a, depayloader, packet, {_header, payload}) do
     with {:ok, result} <- StapA.parse(payload) do
-      nals = Enum.reduce(result, <<>>, fn nal, acc -> acc <> prefix_annexb(nal) end)
+      nals = result |> Stream.map(&prefix_annexb/1) |> Enum.join()
       {:ok, {nals, %__MODULE__{depayloader | current_timestamp: packet.timestamp}}}
     end
   end
 
-  defp handle_unit_type(unsupported_type, _depayloader, _packet, _nal) do
+  defp do_depayload(unsupported_type, _depayloader, _packet, _nal) do
     Logger.warning("""
       Received packet with unsupported NAL type: #{unsupported_type}. Supported types are: Single NALU, STAP-A, FU-A. Dropping packet.
     """)

lib/ex_webrtc/rtp/h264/nal_formats/fu.ex

@@ -2,14 +2,9 @@ defmodule ExWebRTC.RTP.H264.FU do
   @moduledoc """
   Module responsible for parsing H264 Fragmentation Unit.
   """
-  use Bunch
   alias __MODULE__
   alias ExWebRTC.RTP.H264.NAL
 
-  defstruct data: []
-
-  @type t :: %__MODULE__{data: [binary()]}
-
   @doc """
   Parses H264 Fragmentation Unit
 
@@ -18,68 +13,28 @@ defmodule ExWebRTC.RTP.H264.FU do
   In case of last packet `{:ok, {type, data}}` tuple will be returned, where data
   is `NAL Unit` created by concatenating subsequent Fragmentation Units.
   """
-  @spec parse(binary(), t) ::
+  @spec parse(binary(), [binary()]) ::
           {:ok, {binary(), NAL.Header.type()}}
           | {:error, :packet_malformed | :invalid_first_packet}
-          | {:incomplete, t()}
+          | {:incomplete, [binary()]}
   def parse(packet, acc) do
     with {:ok, {header, value}} <- FU.Header.parse(packet) do
       do_parse(header, value, acc)
     end
   end
 
-  @doc """
-  Serialize H264 unit into list of FU-A payloads
-  """
-  @spec serialize(binary(), pos_integer()) :: list(binary()) | {:error, :unit_too_small}
-  def serialize(data, preferred_size) do
-    case data do
-      <<header::1-binary, head::binary-size(preferred_size - 1), rest::binary>> ->
-        <<r::1, nri::2, type::5>> = header
-
-        payload =
-          head
-          |> FU.Header.add_header(1, 0, type)
-          |> NAL.Header.add_header(r, nri, NAL.Header.encode_type(:fu_a))
-
-        [payload | do_serialize(rest, r, nri, type, preferred_size)]
-
-      _data ->
-        {:error, :unit_too_small}
-    end
-  end
-
-  defp do_serialize(data, r, nri, type, preferred_size) do
-    case data do
-      <<head::binary-size(preferred_size - 2), rest::binary>> when byte_size(rest) > 0 ->
-        payload =
-          head
-          |> FU.Header.add_header(0, 0, type)
-          |> NAL.Header.add_header(r, nri, NAL.Header.encode_type(:fu_a))
-
-        [payload] ++ do_serialize(rest, r, nri, type, preferred_size)
-
-      rest ->
-        [
-          rest
-          |> FU.Header.add_header(0, 1, type)
-          |> NAL.Header.add_header(r, nri, NAL.Header.encode_type(:fu_a))
-        ]
-    end
-  end
-
   defp do_parse(header, packet, acc)
 
-  defp do_parse(%FU.Header{start_bit: true}, data, %{data: []} = acc),
-    do: {:incomplete, %__MODULE__{acc | data: [data]}}
+  defp do_parse(%FU.Header{start_bit: true}, data, []),
+    do: {:incomplete, [data]}
 
   defp do_parse(%FU.Header{start_bit: true}, _data, _acc),
     do: {:error, :last_fu_not_finished}
 
-  defp do_parse(%FU.Header{start_bit: false}, _data, %__MODULE__{data: []}),
+  defp do_parse(%FU.Header{start_bit: false}, _data, []),
     do: {:error, :invalid_first_packet}
 
-  defp do_parse(%FU.Header{end_bit: true, type: type}, data, %__MODULE__{data: acc_data}) do
+  defp do_parse(%FU.Header{end_bit: true, type: type}, data, acc_data) do
     result =
       [data | acc_data]
       |> Enum.reverse()
@@ -88,6 +43,6 @@ defmodule ExWebRTC.RTP.H264.FU do
     {:ok, {result, type}}
   end
 
-  defp do_parse(_header, data, %__MODULE__{data: acc_data} = fu),
-    do: {:incomplete, %__MODULE__{fu | data: [data | acc_data]}}
+  defp do_parse(_header, data, acc_data),
+    do: {:incomplete, [data | acc_data]}
 end

lib/ex_webrtc/rtp/h264/nal_formats/stap_a.ex

@@ -24,8 +24,6 @@ defmodule ExWebRTC.RTP.H264.StapA do
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   ```
   """
-  use Bunch
-
   alias ExWebRTC.RTP.H264.NAL
 
   @spec parse(binary()) :: {:ok, [binary()]} | {:error, :packet_malformed}
@@ -39,16 +37,4 @@ defmodule ExWebRTC.RTP.H264.StapA do
     do: do_parse(rest, [nalu | acc])
 
   defp do_parse(_data, _acc), do: {:error, :packet_malformed}
-
-  @spec aggregation_unit_size(binary()) :: pos_integer()
-  def aggregation_unit_size(nalu), do: byte_size(nalu) + 2
-
-  @spec serialize([binary], 0..1, 0..3) :: binary
-  def serialize(payloads, f, nri) do
-    payloads
-    |> Enum.reverse()
-    |> Enum.map(&<<byte_size(&1)::16, &1::binary>>)
-    |> IO.iodata_to_binary()
-    |> NAL.Header.add_header(f, nri, NAL.Header.encode_type(:stap_a))
-  end
 end

lib/ex_webrtc/rtp/h264/nal_header.ex

@@ -44,7 +44,7 @@ defmodule ExWebRTC.RTP.H264.NAL.Header do
   | Reserved | 30-31          |
 
   """
-  @type type :: 1..31
+  @type rbsp_type :: 1..31
   @type supported_types :: :stap_a | :fu_a | :single_nalu
   @type unsupported_types :: :stap_b | :mtap_16 | :mtap_24 | :fu_b
   @type types :: supported_types | unsupported_types | :reserved
@@ -53,7 +53,7 @@ defmodule ExWebRTC.RTP.H264.NAL.Header do
 
   @type t :: %__MODULE__{
           nal_ref_idc: nri(),
-          type: type()
+          type: rbsp_type()
         }
 
   @spec parse_unit_header(binary()) :: {:error, :malformed_data} | {:ok, {t(), binary()}}
@@ -74,7 +74,7 @@ defmodule ExWebRTC.RTP.H264.NAL.Header do
   @doc """
   Adds NAL header to payload
   """
-  @spec add_header(binary(), 0 | 1, nri(), type()) :: binary()
+  @spec add_header(binary(), 0 | 1, nri(), rbsp_type()) :: binary()
   def add_header(payload, f, nri, type),
     do: <<f::1, nri::2, type::5>> <> payload
 
@@ -94,17 +94,4 @@ defmodule ExWebRTC.RTP.H264.NAL.Header do
   defp do_decode_type(29), do: :fu_b
   defp do_decode_type(number) when number in [30, 31], do: :reserved
   defp do_decode_type(_), do: :invalid
-
-  @doc """
-  Encodes given NAL type
-  """
-  @spec encode_type(types()) :: type()
-  def encode_type(:single_nalu), do: 1
-  def encode_type(:stap_a), do: 24
-  def encode_type(:stap_b), do: 25
-  def encode_type(:mtap_16), do: 26
-  def encode_type(:mtap_24), do: 27
-  def encode_type(:fu_a), do: 28
-  def encode_type(:fu_b), do: 29
-  def encode_type(:reserved), do: 30
 end

test/ex_webrtc/rtp/depayloader_test.exs

@@ -83,4 +83,10 @@ defmodule ExWebRTC.RTP.DepayloaderTest do
     assert Depayloader.depayload(depayloader, @packet) ==
              Depayloader.H264.depayload(depayloader, @packet)
   end
+
+  test "returns error if no depayloader exists for given codec" do
+    assert {:error, :no_depayloader_for_codec} =
+             %RTPCodecParameters{payload_type: 45, mime_type: "video/AV1", clock_rate: 90_000}
+             |> Depayloader.new()
+  end
 end