Fix skipping parameters (#44)

* Stopped skipping parameter sets when receiving non-idr nalus * Update fixture_generator
membraneframework · Sep 20, 2023 · db94f61 · db94f61
1 parent a014381
commit db94f61
Show file tree

Hide file tree

Showing 26 changed files with 49 additions and 84 deletions.
diff --git a/README.md b/README.md
@@ -22,7 +22,7 @@ The package can be installed by adding `membrane_h264_plugin` to your list of de
 ```elixir
 def deps do
   [
-	  {:membrane_h264_plugin, "~> 0.7.2"}
+	  {:membrane_h264_plugin, "~> 0.7.3"}
   ]
 end
 ```

diff --git a/lib/membrane_h264_plugin/parser.ex b/lib/membrane_h264_plugin/parser.ex
@@ -287,7 +287,6 @@ defmodule Membrane.H264.Parser do
     {nalus, nalu_parser} = NALuParser.parse_nalus(nalus_payloads, timestamps, state.nalu_parser)
     is_au_aligned = state.mode == :au_aligned
     {access_units, au_splitter} = AUSplitter.split(nalus, is_au_aligned, state.au_splitter)
-    {access_units, state} = skip_improper_aus(access_units, state)
     {actions, state} = prepare_actions_for_aus(access_units, ctx, state)
 
     state = %{
@@ -305,8 +304,7 @@ defmodule Membrane.H264.Parser do
     {last_nalu_payload, nalu_splitter} = NALuSplitter.split(<<>>, true, state.nalu_splitter)
     {last_nalu, nalu_parser} = NALuParser.parse_nalus(last_nalu_payload, state.nalu_parser)
     {maybe_improper_aus, au_splitter} = AUSplitter.split(last_nalu, true, state.au_splitter)
-    {aus, state} = skip_improper_aus(maybe_improper_aus, state)
-    {actions, state} = prepare_actions_for_aus(aus, ctx, state)
+    {actions, state} = prepare_actions_for_aus(maybe_improper_aus, ctx, state)
 
     actions = if stream_format_sent?(actions, ctx), do: actions, else: []
 
@@ -395,22 +393,17 @@ defmodule Membrane.H264.Parser do
     {avc, nalu_length_size}
   end
 
-  defp skip_improper_aus(aus, state) do
-    Enum.flat_map_reduce(aus, state, fn au, state ->
-      has_seen_keyframe? =
-        Enum.all?(au, &(&1.status == :valid)) and Enum.any?(au, &(&1.type == :idr))
+  @spec skip_au?(AUSplitter.access_unit(), state()) :: {boolean(), state()}
+  defp skip_au?(au, state) do
+    has_seen_keyframe? =
+      Enum.all?(au, &(&1.status == :valid)) and Enum.any?(au, &(&1.type == :idr))
 
-      state = %{
-        state
-        | skip_until_keyframe: state.skip_until_keyframe and not has_seen_keyframe?
-      }
+    state = %{
+      state
+      | skip_until_keyframe: state.skip_until_keyframe and not has_seen_keyframe?
+    }
 
-      if Enum.any?(au, &(&1.status == :error)) or state.skip_until_keyframe do
-        {[], state}
-      else
-        {[au], state}
-      end
-    end)
+    {Enum.any?(au, &(&1.status == :error)) or state.skip_until_keyframe, state}
   end
 
   @spec prepare_actions_for_aus(
@@ -424,11 +417,17 @@ defmodule Membrane.H264.Parser do
 
       {{pts, dts}, state} = prepare_timestamps(au, state)
 
-      buffers_actions = [
-        buffer:
-          {:output,
-           wrap_into_buffer(au, pts, dts, state.output_alignment, state.output_stream_structure)}
-      ]
+      {should_skip_au, state} = skip_au?(au, state)
+
+      buffers_actions =
+        if should_skip_au do
+          []
+        else
+          buffers =
+            wrap_into_buffer(au, pts, dts, state.output_alignment, state.output_stream_structure)
+
+          [buffer: {:output, buffers}]
+        end
 
       {stream_format_actions ++ buffers_actions, state}
     end)
@@ -559,19 +558,14 @@ defmodule Membrane.H264.Parser do
           Membrane.Time.t(),
           :au | :nalu,
           stream_structure()
-        ) :: Buffer.t()
+        ) :: Buffer.t() | [Buffer.t()]
   defp wrap_into_buffer(access_unit, pts, dts, :au, output_stream_structure) do
-    metadata = prepare_au_metadata(access_unit)
-
-    buffer =
-      Enum.reduce(access_unit, <<>>, fn nalu, acc ->
-        acc <> NALuParser.get_prefixed_nalu_payload(nalu, output_stream_structure)
-      end)
-      |> then(fn payload ->
-        %Buffer{payload: payload, metadata: metadata, pts: pts, dts: dts}
-      end)
-
-    buffer
+    Enum.reduce(access_unit, <<>>, fn nalu, acc ->
+      acc <> NALuParser.get_prefixed_nalu_payload(nalu, output_stream_structure)
+    end)
+    |> then(fn payload ->
+      %Buffer{payload: payload, metadata: prepare_au_metadata(access_unit), pts: pts, dts: dts}
+    end)
   end
 
   defp wrap_into_buffer(access_unit, pts, dts, :nalu, output_stream_structure) do

diff --git a/mix.exs b/mix.exs
@@ -1,7 +1,7 @@
 defmodule Membrane.H264.Plugin.Mixfile do
   use Mix.Project
 
-  @version "0.7.2"
+  @version "0.7.3"
   @github_url "https://github.com/membraneframework-labs/membrane_h264_plugin"
 
   def project do

diff --git a/mix.lock b/mix.lock
@@ -4,17 +4,17 @@
   "coerce": {:hex, :coerce, "1.0.1", "211c27386315dc2894ac11bc1f413a0e38505d808153367bd5c6e75a4003d096", [:mix], [], "hexpm", "b44a691700f7a1a15b4b7e2ff1fa30bebd669929ac8aa43cffe9e2f8bf051cf1"},
   "credo": {:hex, :credo, "1.7.0", "6119bee47272e85995598ee04f2ebbed3e947678dee048d10b5feca139435f75", [:mix], [{:bunt, "~> 0.2.1", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2.8", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "6839fcf63d1f0d1c0f450abc8564a57c43d644077ab96f2934563e68b8a769d7"},
   "dialyxir": {:hex, :dialyxir, "1.4.1", "a22ed1e7bd3a3e3f197b68d806ef66acb61ee8f57b3ac85fc5d57354c5482a93", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "84b795d6d7796297cca5a3118444b80c7d94f7ce247d49886e7c291e1ae49801"},
-  "earmark_parser": {:hex, :earmark_parser, "1.4.33", "3c3fd9673bb5dcc9edc28dd90f50c87ce506d1f71b70e3de69aa8154bc695d44", [:mix], [], "hexpm", "2d526833729b59b9fdb85785078697c72ac5e5066350663e5be6a1182da61b8f"},
+  "earmark_parser": {:hex, :earmark_parser, "1.4.35", "437773ca9384edf69830e26e9e7b2e0d22d2596c4a6b17094a3b29f01ea65bb8", [:mix], [], "hexpm", "8652ba3cb85608d0d7aa2d21b45c6fad4ddc9a1f9a1f1b30ca3a246f0acc33f6"},
   "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
   "ex_doc": {:hex, :ex_doc, "0.30.6", "5f8b54854b240a2b55c9734c4b1d0dd7bdd41f71a095d42a70445c03cf05a281", [:mix], [{:earmark_parser, "~> 1.4.31", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "bd48f2ddacf4e482c727f9293d9498e0881597eae6ddc3d9562bd7923375109f"},
   "file_system": {:hex, :file_system, "0.2.10", "fb082005a9cd1711c05b5248710f8826b02d7d1784e7c3451f9c1231d4fc162d", [:mix], [], "hexpm", "41195edbfb562a593726eda3b3e8b103a309b733ad25f3d642ba49696bf715dc"},
   "jason": {:hex, :jason, "1.4.1", "af1504e35f629ddcdd6addb3513c3853991f694921b1b9368b0bd32beb9f1b63", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "fbb01ecdfd565b56261302f7e1fcc27c4fb8f32d56eab74db621fc154604a7a1"},
   "makeup": {:hex, :makeup, "1.1.0", "6b67c8bc2882a6b6a445859952a602afc1a41c2e08379ca057c0f525366fc3ca", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "0a45ed501f4a8897f580eabf99a2e5234ea3e75a4373c8a52824f6e873be57a6"},
   "makeup_elixir": {:hex, :makeup_elixir, "0.16.1", "cc9e3ca312f1cfeccc572b37a09980287e243648108384b97ff2b76e505c3555", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "e127a341ad1b209bd80f7bd1620a15693a9908ed780c3b763bccf7d200c767c6"},
   "makeup_erlang": {:hex, :makeup_erlang, "0.1.2", "ad87296a092a46e03b7e9b0be7631ddcf64c790fa68a9ef5323b6cbb36affc72", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "f3f5a1ca93ce6e092d92b6d9c049bcda58a3b617a8d888f8e7231c85630e8108"},
-  "membrane_core": {:hex, :membrane_core, "0.12.8", "59fdd10a1c1c6757302748d029fba4936257e53c93ac49fddd094962c08180f9", [:mix], [{:bunch, "~> 1.6", [hex: :bunch, repo: "hexpm", optional: false]}, {:qex, "~> 0.3", [hex: :qex, repo: "hexpm", optional: false]}, {:ratio, "~> 2.0", [hex: :ratio, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "d48ab1adc13d5820182b016cc397692ed5568d4064b5765c343b4d64c7f119e7"},
+  "membrane_core": {:hex, :membrane_core, "0.12.9", "b80239deacf98f24cfd2e0703b632e92ddded8b989227cd6e724140f433b0aac", [:mix], [{:bunch, "~> 1.6", [hex: :bunch, repo: "hexpm", optional: false]}, {:qex, "~> 0.3", [hex: :qex, repo: "hexpm", optional: false]}, {:ratio, "~> 2.0", [hex: :ratio, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "389b4b22da0e35d5b053ec2fa87bf36882e0ab88f8fb841af895982fb4abe504"},
   "membrane_file_plugin": {:hex, :membrane_file_plugin, "0.14.0", "87f19f5f5afbfbaf2219b8f1d8496534cb9ad01fca74687910bf3f7aa866e244", [:mix], [{:membrane_core, "~> 0.12.0", [hex: :membrane_core, repo: "hexpm", optional: false]}], "hexpm", "28956f8d5d87735d499c57f1c24f62aeab71e0211863759e7e695ead966eb433"},
-  "membrane_h264_format": {:hex, :membrane_h264_format, "0.6.0", "8f3b920c3c7d73c24dce1e39b636b660645e43ab5d3e5913ac022cfad6c7b6b8", [:mix], [], "hexpm", "d3cc012bec8c70d4a5483429da75c4c1fc9b079d8816721763bb98d9fcb44fd6"},
+  "membrane_h264_format": {:hex, :membrane_h264_format, "0.6.1", "44836cd9de0abe989b146df1e114507787efc0cf0da2368f17a10c47b4e0738c", [:mix], [], "hexpm", "4b79be56465a876d2eac2c3af99e115374bbdc03eb1dea4f696ee9a8033cd4b0"},
   "membrane_stream_plugin": {:hex, :membrane_stream_plugin, "0.3.1", "265f327dad815a8ef18f8872bdebc6c1aa1fea3d059446c937b76ff067c166e6", [:mix], [{:membrane_core, "~> 0.12.0", [hex: :membrane_core, repo: "hexpm", optional: false]}], "hexpm", "b504296b9f5a78b43c0d46ad66202a1f0f93d4c122949c27ad4c04f67f3c5907"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.3.1", "2c54013ecf170e249e9291ed0a62e5832f70a476c61da16f6aac6dca0189f2af", [:mix], [], "hexpm", "2682e3c0b2eb58d90c6375fc0cc30bc7be06f365bf72608804fb9cffa5e1b167"},
   "numbers": {:hex, :numbers, "5.2.4", "f123d5bb7f6acc366f8f445e10a32bd403c8469bdbce8ce049e1f0972b607080", [:mix], [{:coerce, "~> 1.0", [hex: :coerce, repo: "hexpm", optional: false]}, {:decimal, "~> 1.9 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "eeccf5c61d5f4922198395bf87a465b6f980b8b862dd22d28198c5e6fab38582"},

diff --git a/test/fixtures/ms/ref_video-avc1-au.ms b/test/fixtures/ms/ref_video-avc1-au.ms
diff --git a/test/fixtures/ms/ref_video-avc1-nalu.ms b/test/fixtures/ms/ref_video-avc1-nalu.ms
diff --git a/test/fixtures/ms/ref_video-avc3-au.ms b/test/fixtures/ms/ref_video-avc3-au.ms
diff --git a/test/fixtures/ms/ref_video-avc3-nalu.ms b/test/fixtures/ms/ref_video-avc3-nalu.ms
diff --git a/test/fixtures/ms/ref_video_fast_start-avc1-au.ms b/test/fixtures/ms/ref_video_fast_start-avc1-au.ms
diff --git a/test/fixtures/ms/ref_video_fast_start-avc1-nalu.ms b/test/fixtures/ms/ref_video_fast_start-avc1-nalu.ms
diff --git a/test/fixtures/ms/ref_video_fast_start-avc3-au.ms b/test/fixtures/ms/ref_video_fast_start-avc3-au.ms
diff --git a/test/fixtures/ms/ref_video_fast_start-avc3-nalu.ms b/test/fixtures/ms/ref_video_fast_start-avc3-nalu.ms
diff --git a/test/fixtures/ms/ref_video_variable_parameters-avc3-au.ms b/test/fixtures/ms/ref_video_variable_parameters-avc3-au.ms
diff --git a/test/fixtures/ms/ref_video_variable_parameters-avc3-nalu.ms b/test/fixtures/ms/ref_video_variable_parameters-avc3-nalu.ms
diff --git a/test/fixtures/msr/ref_video-avc1-au.msr b/test/fixtures/msr/ref_video-avc1-au.msr
diff --git a/test/fixtures/msr/ref_video-avc1-nalu.msr b/test/fixtures/msr/ref_video-avc1-nalu.msr
diff --git a/test/fixtures/msr/ref_video-avc3-au.msr b/test/fixtures/msr/ref_video-avc3-au.msr
diff --git a/test/fixtures/msr/ref_video-avc3-nalu.msr b/test/fixtures/msr/ref_video-avc3-nalu.msr
diff --git a/test/fixtures/msr/ref_video_fast_start-avc1-au.msr b/test/fixtures/msr/ref_video_fast_start-avc1-au.msr
diff --git a/test/fixtures/msr/ref_video_fast_start-avc1-nalu.msr b/test/fixtures/msr/ref_video_fast_start-avc1-nalu.msr
diff --git a/test/fixtures/msr/ref_video_fast_start-avc3-au.msr b/test/fixtures/msr/ref_video_fast_start-avc3-au.msr
diff --git a/test/fixtures/msr/ref_video_fast_start-avc3-nalu.msr b/test/fixtures/msr/ref_video_fast_start-avc3-nalu.msr
diff --git a/test/fixtures/msr/ref_video_variable_parameters-avc3-au.msr b/test/fixtures/msr/ref_video_variable_parameters-avc3-au.msr
diff --git a/test/fixtures/msr/ref_video_variable_parameters-avc3-nalu.msr b/test/fixtures/msr/ref_video_variable_parameters-avc3-nalu.msr
diff --git a/test/parser/stream_structure_conversion_test.exs b/test/parser/stream_structure_conversion_test.exs
@@ -15,10 +15,10 @@ defmodule Membrane.H264.StreamStructureConversionTest do
                    |> Path.wildcard()
                    |> Enum.reject(&String.contains?(&1, ["no-sps", "no-pps", "sps-pps-non-idr"]))
 
-  @avc1_au_fixtures "../fixtures/ms/*-avc1-au.ms" |> Path.expand(__DIR__) |> Path.wildcard()
-  @avc1_nalu_fixtures "../fixtures/ms/*-avc1-nalu.ms" |> Path.expand(__DIR__) |> Path.wildcard()
-  @avc3_au_fixtures "../fixtures/ms/*-avc3-au.ms" |> Path.expand(__DIR__) |> Path.wildcard()
-  @avc3_nalu_fixtures "../fixtures/ms/*-avc3-nalu.ms" |> Path.expand(__DIR__) |> Path.wildcard()
+  @avc1_au_fixtures "../fixtures/msr/*-avc1-au.msr" |> Path.expand(__DIR__) |> Path.wildcard()
+  @avc1_nalu_fixtures "../fixtures/msr/*-avc1-nalu.msr" |> Path.expand(__DIR__) |> Path.wildcard()
+  @avc3_au_fixtures "../fixtures/msr/*-avc3-au.msr" |> Path.expand(__DIR__) |> Path.wildcard()
+  @avc3_nalu_fixtures "../fixtures/msr/*-avc3-nalu.msr" |> Path.expand(__DIR__) |> Path.wildcard()
 
   defp make_annexb_pipeline(alignment, parsers) do
     parser_chain = make_parser_chain(parsers)

diff --git a/test/support/fixture_generator.exs b/test/support/fixture_generator.exs
@@ -1,35 +1,20 @@
 Mix.install([
-  {:membrane_file_plugin, "~> 0.14.0"},
-  {:membrane_hackney_plugin, "~> 0.10.0"},
-  {:membrane_mp4_plugin, "~> 0.25.0"},
-  {:membrane_mp4_format, ">= 0.0.0"},
-  {:membrane_stream_plugin, "~> 0.3.1"},
-  {:membrane_aac_plugin, ">= 0.0.0"},
-  {:membrane_h264_format,
-   github: "membraneframework/membrane_h264_format", ref: "ea5a3d2", override: true},
-  {:membrane_h264_plugin,
-   github: "membraneframework/membrane_h264_plugin",
-   branch: "stream-type-conversion",
-   override: true}
+  {:membrane_file_plugin, "~> 0.15.0"},
+  {:membrane_mp4_plugin, "~> 0.29.0"},
+  {:membrane_stream_plugin, "~> 0.3.1"}
 ])
 
 alias Membrane.H264.Parser.{NALuSplitter, DecoderConfigurationRecord}
 
-defmodule Membrane.H264.RemoteStream do
-  @moduledoc false
-
-  defstruct []
-end
-
-defmodule MP4ToH264Filter do
+defmodule Aligner do
   @moduledoc false
 
   use Membrane.Filter
 
   def_input_pad :input,
     demand_unit: :buffers,
     demand_mode: :auto,
-    accepted_format: Membrane.MP4.Payload
+    accepted_format: Membrane.H264
 
   def_output_pad :output,
     demand_mode: :auto,
@@ -43,23 +28,10 @@ defmodule MP4ToH264Filter do
                 spec: {:avc1 | :avc3, pos_integer()}
               ]
 
-  @impl true
-  def handle_init(_ctx, opts) do
-    {[],
-     %{
-       output_alignment: opts.output_alignment,
-       output_stream_structure: opts.output_stream_structure
-     }}
-  end
-
   @impl true
   def handle_stream_format(
         :input,
-        %Membrane.MP4.Payload{
-          width: width,
-          height: height,
-          content: %Membrane.MP4.Payload.AVC1{avcc: dcr}
-        },
+        %Membrane.H264{stream_structure: {:avc1, dcr}} = stream_format,
         _ctx,
         %{output_stream_structure: {avc, nalu_length_size}} = state
       ) do
@@ -73,10 +45,9 @@ defmodule MP4ToH264Filter do
        stream_format:
          {:output,
           %Membrane.H264{
-            width: width,
-            height: height,
-            alignment: state.output_alignment,
-            stream_structure: {avc, dcr}
+            stream_format
+            | alignment: state.output_alignment,
+              stream_structure: {avc, dcr}
           }}
      ], state}
   end
@@ -117,7 +88,7 @@ defmodule FixtureGeneratorPipeline do
       child(:video_source, %Membrane.File.Source{location: options.input_location})
       |> child(:demuxer, Membrane.MP4.Demuxer.ISOM)
       |> via_out(Pad.ref(:output, 1))
-      |> child(:filter, %MP4ToH264Filter{
+      |> child(:filter, %Aligner{
         output_alignment: options.output_alignment,
         output_stream_structure: options.stream_structure
       })
@@ -172,10 +143,10 @@ defmodule AVCFixtureGenerator do
     output_location =
       input_location
       |> Path.split()
-      |> List.replace_at(-2, "ms")
+      |> List.replace_at(-2, "msr")
       |> List.update_at(-1, fn file ->
         [name, "mp4"] = String.split(file, ".")
-        "#{name}-#{avc}-#{output_alignment}.ms"
+        "#{name}-#{avc}-#{output_alignment}.msr"
       end)
       |> Path.join()