Skip to content

Commit

Permalink
remove sequence number validation in depayloader, rely on discontinui…
Browse files Browse the repository at this point in the history
…ty (#53)

* remove sequence number validation in depayloader, rely on discontinuity

* bump version
  • Loading branch information
mat-hek authored Oct 30, 2024
1 parent 894cbd0 commit c2b5e6c
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 52 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ The package can be installed by adding `membrane_rtp_h264_plugin` to your list o
```elixir
def deps do
[
{:membrane_rtp_h264_plugin, "~> 0.19.4"}
{:membrane_rtp_h264_plugin, "~> 0.20.0"}
]
end
```
Expand Down
4 changes: 1 addition & 3 deletions lib/rtp_h264/depayloader.ex
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ defmodule Membrane.RTP.H264.Depayloader do
end

defp handle_unit_type(:fu_a, {header, data}, buffer, state) do
%Buffer{metadata: %{rtp: %{sequence_number: seq_num}}} = buffer

case FU.parse(data, seq_num, map_state_to_fu(state)) do
case FU.parse(data, map_state_to_fu(state)) do
{:ok, {data, type}} ->
data = NAL.Header.add_header(data, 0, header.nal_ref_idc, type)
result = buffer_output(data, buffer, %State{state | parser_acc: nil})
Expand Down
40 changes: 14 additions & 26 deletions lib/rtp_h264/nal_formats/fu.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,9 @@ defmodule Membrane.RTP.H264.FU do
alias __MODULE__
alias Membrane.RTP.H264.NAL

defstruct [:last_seq_num, data: []]
defstruct data: []

@type t :: %__MODULE__{
data: [binary()],
last_seq_num: nil | non_neg_integer()
}

defguardp is_next(last_seq_num, next_seq_num) when rem(last_seq_num + 1, 65_536) == next_seq_num
@type t :: %__MODULE__{data: [binary()]}

@doc """
Parses H264 Fragmentation Unit
Expand All @@ -23,13 +18,13 @@ defmodule Membrane.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(), non_neg_integer(), t) ::
@spec parse(binary(), t) ::
{:ok, {binary(), NAL.Header.type()}}
| {:error, :packet_malformed | :invalid_first_packet}
| {:incomplete, t()}
def parse(data, seq_num, acc) do
with {:ok, {header, value}} <- FU.Header.parse(data) do
do_parse(header, value, seq_num, acc)
def parse(packet, acc) do
with {:ok, {header, value}} <- FU.Header.parse(packet) do
do_parse(header, value, acc)
end
end

Expand Down Expand Up @@ -73,30 +68,23 @@ defmodule Membrane.RTP.H264.FU do
end
end

defp do_parse(header, data, seq_num, acc)
defp do_parse(header, packet, acc)

defp do_parse(%FU.Header{start_bit: true}, data, seq_num, acc),
do: {:incomplete, %__MODULE__{acc | data: [data], last_seq_num: seq_num}}
defp do_parse(%FU.Header{start_bit: true}, packet, acc),
do: {:incomplete, %__MODULE__{acc | data: [packet]}}

defp do_parse(%FU.Header{start_bit: false}, _data, _seq_num, %__MODULE__{last_seq_num: nil}),
defp do_parse(%FU.Header{start_bit: false}, _data, %__MODULE__{data: []}),
do: {:error, :invalid_first_packet}

defp do_parse(%FU.Header{end_bit: true, type: type}, data, seq_num, %__MODULE__{
data: acc,
last_seq_num: last
})
when is_next(last, seq_num) do
defp do_parse(%FU.Header{end_bit: true, type: type}, packet, %__MODULE__{data: acc_data}) do
result =
[data | acc]
[packet | acc_data]
|> Enum.reverse()
|> Enum.join()

{:ok, {result, type}}
end

defp do_parse(_header, data, seq_num, %__MODULE__{data: acc, last_seq_num: last} = fu)
when is_next(last, seq_num),
do: {:incomplete, %__MODULE__{fu | data: [data | acc], last_seq_num: seq_num}}

defp do_parse(_header, _data, _seq_num, _fu), do: {:error, :missing_packet}
defp do_parse(_header, packet, %__MODULE__{data: acc_data} = fu),
do: {:incomplete, %__MODULE__{fu | data: [packet | acc_data]}}
end
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
defmodule Membrane.RTP.H264.MixProject do
use Mix.Project

@version "0.19.4"
@version "0.20.0"
@github_url "https://github.com/membraneframework/membrane_rtp_h264_plugin"

def project do
Expand Down
26 changes: 5 additions & 21 deletions test/nal_formaters/fu_a_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,50 +5,34 @@ defmodule Membrane.RTP.H264.FUTest do
alias Membrane.RTP.H264.FU
alias Membrane.Support.Formatters.FUFactory

@base_seq_num 4567

describe "Fragmentation Unit parser" do
test "parses first packet" do
packet = FUFactory.first()

assert {:incomplete, fu} = FU.parse(packet, @base_seq_num, %FU{})
assert %FU{last_seq_num: @base_seq_num} = fu
assert {:incomplete, _fu} = FU.parse(packet, %FU{})
end

test "parses packet sequence" do
fixtures = FUFactory.get_all_fixtures()

result =
fixtures
|> Enum.zip(1..Enum.count(fixtures))
|> Enum.reduce(%FU{}, fn {elem, seq_num}, acc ->
FU.parse(elem, seq_num, acc)
|> Enum.reduce(%FU{}, fn elem, acc ->
FU.parse(elem, acc)
~> ({_command, value} -> value)
end)

expected_result = FUFactory.glued_fixtures() ~> (<<_header::8, rest::binary>> -> rest)
assert result == {expected_result, 1}
end

test "returns error when one of non edge packets dropped" do
fixtures = FUFactory.get_all_fixtures()

assert {:error, :missing_packet} ==
fixtures
|> Enum.zip([0, 1, 3, 4, 5])
|> Enum.reduce(%FU{}, fn {elem, seq_num}, acc ->
FU.parse(elem, seq_num, acc)
~>> ({command, fu} when command in [:incomplete, :ok] -> fu)
end)
end

test "returns error when first packet is not starting packet" do
invalid_first_packet = <<0::5, 3::3>>
assert {:error, :invalid_first_packet} == FU.parse(invalid_first_packet, 2, %FU{})
assert {:error, :invalid_first_packet} == FU.parse(invalid_first_packet, %FU{})
end

test "returns error when header is not valid" do
assert {:error, :packet_malformed} == FU.parse(<<0::2, 1::1, 1::5>>, 0, %FU{})
assert {:error, :packet_malformed} == FU.parse(<<0::2, 1::1, 1::5>>, %FU{})
end
end
end

0 comments on commit c2b5e6c

Please sign in to comment.