Fix parsing esds (#30)

* Fix parsing section 5 of esds

* Remove code duplication of parsing and generating AudioSpecificConfig

Co-authored-by: Bartosz Błaszków <[email protected]>

README.md

@@ -15,7 +15,7 @@ The package can be installed by adding `membrane_aac_plugin` to your list of dep
 ```elixir
 def deps do
   [
-    {:membrane_aac_plugin, "~> 0.16.0"}
+    {:membrane_aac_plugin, "~> 0.16.1"}
   ]
 end
 ```

lib/membrane/aac/parser/audio_specific_config.ex

@@ -11,18 +11,16 @@ defmodule Membrane.AAC.Parser.AudioSpecificConfig do
   def generate_audio_specific_config(stream_format) do
     aot = AAC.profile_to_aot_id(stream_format.profile)
     frequency_id = AAC.sample_rate_to_sampling_frequency_id(stream_format.sample_rate)
-    channel_configuration = AAC.channels_to_channel_config_id(stream_format.channels)
+    channel_config_id = AAC.channels_to_channel_config_id(stream_format.channels)
+    frame_length_id = AAC.samples_per_frame_to_frame_length_id(stream_format.samples_per_frame)
 
-    frame_length_flag =
-      case stream_format.samples_per_frame do
-        960 -> 1
-        1024 -> 0
-      end
+    depends_on_core_coder = 0
+    extension_flag = 0
 
     custom_frequency = if frequency_id == 15, do: <<stream_format.sample_rate::24>>, else: <<>>
 
-    <<aot::5, frequency_id::4, custom_frequency::binary, channel_configuration::4,
-      frame_length_flag::1, 0::1, 0::1>>
+    <<aot::5, frequency_id::4, custom_frequency::binary, channel_config_id::4, frame_length_id::1,
+      depends_on_core_coder::1, extension_flag::1>>
   end
 
   @spec parse_audio_specific_config(binary()) :: AAC.t()
@@ -31,8 +29,8 @@ defmodule Membrane.AAC.Parser.AudioSpecificConfig do
 
     custom_frequency_length = if frequency_id == 15, do: 24, else: 0
 
-    <<custom_frequency::integer-size(custom_frequency_length), channel_configuration::4,
-      frame_length_flag::1, _rest::bits>> = audio_specific_config_rest
+    <<custom_frequency::integer-size(custom_frequency_length), channel_config_id::4,
+      frame_length_id::1, _rest::bits>> = audio_specific_config_rest
 
     sample_rate =
       if frequency_id == 15,
@@ -43,9 +41,10 @@ defmodule Membrane.AAC.Parser.AudioSpecificConfig do
       profile: AAC.aot_id_to_profile(profile),
       mpeg_version: 4,
       sample_rate: sample_rate,
-      channels: AAC.channel_config_id_to_channels(channel_configuration),
+      channels: AAC.channel_config_id_to_channels(channel_config_id),
       encapsulation: :none,
-      samples_per_frame: if(frame_length_flag == 0, do: 1024, else: 960)
+      samples_per_frame: AAC.frame_length_id_to_samples_per_frame(frame_length_id),
+      config: {:audio_specific_config, audio_specific_config}
     }
   end
 end

lib/membrane/aac/parser/config.ex

@@ -1,6 +1,7 @@
 defmodule Membrane.AAC.Parser.Config do
   @moduledoc false
 
+  require Membrane.Logger
   alias Membrane.AAC
   alias Membrane.AAC.Parser.{AudioSpecificConfig, Esds}
 
@@ -23,10 +24,25 @@ defmodule Membrane.AAC.Parser.Config do
   def parse_config(stream_format) do
     case stream_format.config do
       {:esds, esds} ->
-        Map.merge(stream_format, Esds.parse_esds(esds))
+        esds_stream_format = Esds.parse_esds(esds)
 
-      {:audio_specific_config, config} ->
-        AudioSpecificConfig.parse_audio_specific_config(config)
+        [:sample_rate, :channels]
+        |> Enum.map(fn field ->
+          {field, Map.fetch!(esds_stream_format, field), Map.fetch!(stream_format, field)}
+        end)
+        |> Enum.filter(fn {_field, esds_field, stream_field} -> esds_field != stream_field end)
+        |> Enum.each(fn {field, esds_field, stream_field} ->
+          Membrane.Logger.warning("""
+          #{inspect(field)} decoded from esds differs from stream_format:
+          esds: #{inspect(esds_field)}
+          stream_format: #{inspect(stream_field)}
+          """)
+        end)
+
+        esds_stream_format
+
+      {:audio_specific_config, audio_specific_config} ->
+        AudioSpecificConfig.parse_audio_specific_config(audio_specific_config)
 
       nil ->
         stream_format

lib/membrane/aac/parser/esds.ex

@@ -6,25 +6,12 @@ defmodule Membrane.AAC.Parser.Esds do
   """
 
   alias Membrane.AAC
+  alias Membrane.AAC.Parser.AudioSpecificConfig
 
   @spec generate_esds(AAC.t(), Membrane.Element.state()) :: binary()
   def generate_esds(stream_format, state) do
-    aot_id = Membrane.AAC.profile_to_aot_id(stream_format.profile)
-    frequency_id = Membrane.AAC.sample_rate_to_sampling_frequency_id(stream_format.sample_rate)
-    channel_config_id = Membrane.AAC.channels_to_channel_config_id(stream_format.channels)
-
-    frame_length_id =
-      Membrane.AAC.samples_per_frame_to_frame_length_id(stream_format.samples_per_frame)
-
-    depends_on_core_coder = 0
-    extension_flag = 0
-
-    custom_frequency = if frequency_id == 15, do: <<stream_format.sample_rate::24>>, else: <<>>
-
     section5 =
-      <<aot_id::5, frequency_id::4, custom_frequency::binary, channel_config_id::4,
-        frame_length_id::1, depends_on_core_coder::1, extension_flag::1>>
-      |> generate_esds_section(5)
+      generate_esds_section(AudioSpecificConfig.generate_audio_specific_config(stream_format), 5)
 
     # 64 = mpeg4-audio
     object_type_id = 64
@@ -53,16 +40,12 @@ defmodule Membrane.AAC.Parser.Esds do
     <<section_no, type_tag::binary, byte_size(payload), payload::binary>>
   end
 
-  @spec parse_esds(binary()) :: %{
-          profile: AAC.profile(),
-          mpeg_version: 4,
-          samples_per_frame: AAC.samples_per_frame()
-        }
+  @spec parse_esds(binary()) :: AAC.t()
   def parse_esds(esds) do
     stream_priority = 0
 
-    {esds, <<>>} = unpack_esds_section(esds, 3)
-    <<_elementary_stream_id::16, ^stream_priority, rest::binary>> = esds
+    {section_3, <<>>} = unpack_esds_section(esds, 3)
+    <<_elementary_stream_id::16, ^stream_priority, rest::binary>> = section_3
 
     {section_4, esds_section_6} = unpack_esds_section(rest, 4)
     {<<2>>, <<>>} = unpack_esds_section(esds_section_6, 6)
@@ -80,21 +63,8 @@ defmodule Membrane.AAC.Parser.Esds do
 
     {section_5, <<>>} = unpack_esds_section(esds_section_5, 5)
 
-    depends_on_core_coder = 0
-    extension_flag = 0
-
-    <<aot_id::5, frequency_id::4, section_5_rest::bitstring>> = section_5
-
-    custom_frequency_length = if frequency_id == 15, do: 24, else: 0
-
-    <<_maybe_custom_frequency::integer-size(custom_frequency_length), _channel_config_id::4,
-      frame_length_id::1, ^depends_on_core_coder::1, ^extension_flag::1>> = section_5_rest
-
-    %{
-      profile: AAC.aot_id_to_profile(aot_id),
-      mpeg_version: 4,
-      samples_per_frame: AAC.frame_length_id_to_samples_per_frame(frame_length_id)
-    }
+    AudioSpecificConfig.parse_audio_specific_config(section_5)
+    |> Map.put(:config, {:esds, esds})
   end
 
   defp unpack_esds_section(section, section_no) do

mix.exs

@@ -1,7 +1,7 @@
 defmodule Membrane.AAC.MixProject do
   use Mix.Project
 
-  @version "0.16.0"
+  @version "0.16.1"
   @github_url "https://github.com/membraneframework/membrane_aac_plugin"
 
   def project do

mix.lock

@@ -5,18 +5,18 @@
   "coerce": {:hex, :coerce, "1.0.1", "211c27386315dc2894ac11bc1f413a0e38505d808153367bd5c6e75a4003d096", [:mix], [], "hexpm", "b44a691700f7a1a15b4b7e2ff1fa30bebd669929ac8aa43cffe9e2f8bf051cf1"},
   "crc": {:hex, :crc, "0.10.5", "ee12a7c056ac498ef2ea985ecdc9fa53c1bfb4e53a484d9f17ff94803707dfd8", [:mix, :rebar3], [{:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "3e673b6495a9525c5c641585af1accba59a1eb33de697bedf341e247012c2c7f"},
   "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.3.0", "fd1672f0922b7648ff9ce7b1b26fcf0ef56dda964a459892ad15f6b4410b5284", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "00b2a4bcd6aa8db9dcb0b38c1225b7277dca9bc370b6438715667071a304696f"},
+  "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"},
   "elixir_make": {:hex, :elixir_make, "0.7.7", "7128c60c2476019ed978210c245badf08b03dbec4f24d05790ef791da11aa17c", [:mix], [{:castore, "~> 0.1 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}], "hexpm", "5bc19fff950fad52bbe5f211b12db9ec82c6b34a9647da0c2224b8b8464c7e6c"},
   "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
-  "ex_doc": {:hex, :ex_doc, "0.30.5", "aa6da96a5c23389d7dc7c381eba862710e108cee9cfdc629b7ec021313900e9e", [: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", "88a1e115dcb91cefeef7e22df4a6ebbe4634fbf98b38adcbc25c9607d6d9d8e6"},
+  "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_aac_format": {:hex, :membrane_aac_format, "0.8.0", "515631eabd6e584e0e9af2cea80471fee6246484dbbefc4726c1d93ece8e0838", [:mix], [{:bimap, "~> 1.1", [hex: :bimap, repo: "hexpm", optional: false]}], "hexpm", "a30176a94491033ed32be45e51d509fc70a5ee6e751f12fd6c0d60bd637013f6"},
-  "membrane_core": {:hex, :membrane_core, "0.12.7", "9d3dd564e32768919c1105b4579bd2eef12df7473da5d789185544ae22610e2d", [: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", "321e4009b7068ca04b65daf5c79b8c3772d4286c27d05e50939ec6d9b4d50e59"},
+  "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_file_plugin": {:hex, :membrane_file_plugin, "0.13.3", "aaf40a72e5fccf6da47ec85ef525234acbec828425f1300f74c464bca1487f40", [:mix], [{:membrane_core, "~> 0.11", [hex: :membrane_core, repo: "hexpm", optional: false]}], "hexpm", "1c1acf610d4fc0279b7fd65a5db06c8bc3ef6a276bf40fb033c2c735c25839ba"},
   "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"},