Fix non-fast-start files demuxer bug

README.md

@@ -12,7 +12,7 @@ The package can be installed by adding `membrane_mp4_plugin` to your list of dep
 ```elixir
 defp deps do
   [
-    {:membrane_mp4_plugin, "~> 0.29.0"}
+    {:membrane_mp4_plugin, "~> 0.29.1"}
   ]
 end
 ```

examples/demuxer_isom.exs

@@ -18,7 +18,7 @@ defmodule Example do
         location: @input_file,
         hackney_opts: [follow_redirect: true]
       })
-      |> child(:demuxer, %Membrane.MP4.Demuxer.ISOM{optimize_for_non_fast_start?: true})
+      |> child(:demuxer, Membrane.MP4.Demuxer.ISOM)
       |> via_out(Pad.ref(:output, 1))
       |> child(:parser_video, %Membrane.H264.Parser{output_stream_structure: :annexb})
       |> child(:sink_video, %Membrane.File.Sink{location: @output_video}),
@@ -39,7 +39,7 @@ defmodule Example do
     state = %{state | children_with_eos: MapSet.put(state.children_with_eos, element)}
 
     actions =
-      if Enum.all?([:sink_video, :sink_audio], &(&1 in state.children_with_eos)),
+      if Enum.all?([:sink_audio], &(&1 in state.children_with_eos)),
         do: [terminate: :shutdown],
         else: []
 

lib/membrane_mp4/demuxer/isom.ex

@@ -237,7 +237,8 @@ defmodule Membrane.MP4.Demuxer.ISOM do
       ctx == :started_parsing_mdat and all_headers_read? ->
         :mdat_reading
 
-      ctx == :started_parsing_mdat and not all_headers_read? ->
+      ctx == :started_parsing_mdat and not all_headers_read? and
+          state.optimize_for_non_fast_start? ->
         :skip_mdat
 
       true ->

mix.exs

@@ -1,7 +1,7 @@
 defmodule Membrane.MP4.Plugin.MixProject do
   use Mix.Project
 
-  @version "0.29.0"
+  @version "0.29.1"
   @github_url "https://github.com/membraneframework/membrane_mp4_plugin"
 
   def project do

mix.lock

@@ -7,11 +7,11 @@
   "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"},

test/membrane_mp4/demuxer/isom/demuxer_test.exs

@@ -23,7 +23,7 @@ defmodule Membrane.MP4.Demuxer.ISOM.DemuxerTest do
 
   describe "Demuxer should allow for transmuxing of" do
     @tag :tmp_dir
-    test "a single H264 track", %{tmp_dir: dir} do
+    test "a single fast start H264 track", %{tmp_dir: dir} do
       in_path = "test/fixtures/isom/ref_video_fast_start.mp4"
       out_path = Path.join(dir, "out")
 
@@ -37,7 +37,7 @@ defmodule Membrane.MP4.Demuxer.ISOM.DemuxerTest do
     end
 
     @tag :tmp_dir
-    test "a single AAC track", %{tmp_dir: dir} do
+    test "a single fast start AAC track", %{tmp_dir: dir} do
       in_path = "test/fixtures/isom/ref_aac_fast_start.mp4"
       out_path = Path.join(dir, "out")
 
@@ -49,6 +49,34 @@ defmodule Membrane.MP4.Demuxer.ISOM.DemuxerTest do
 
       perform_test(pipeline, "aac", out_path)
     end
+
+    @tag :tmp_dir
+    test "a single non-fast-start H264 track", %{tmp_dir: dir} do
+      in_path = "test/fixtures/isom/ref_video.mp4"
+      out_path = Path.join(dir, "out")
+
+      pipeline =
+        start_testing_pipeline!(
+          input_file: in_path,
+          output_file: out_path
+        )
+
+      perform_test(pipeline, "video", out_path)
+    end
+
+    @tag :tmp_dir
+    test "a single non-fast-start AAC track", %{tmp_dir: dir} do
+      in_path = "test/fixtures/isom/ref_aac.mp4"
+      out_path = Path.join(dir, "out")
+
+      pipeline =
+        start_testing_pipeline!(
+          input_file: in_path,
+          output_file: out_path
+        )
+
+      perform_test(pipeline, "aac", out_path)
+    end
   end
 
   describe "Demuxer with `non_fast_start_optimization: true` should allow for demuxing" do

test/membrane_mp4/demuxer/isom/integration_test.exs

@@ -11,125 +11,33 @@ defmodule Membrane.MP4.Demuxer.ISOM.IntegrationTest do
 
   describe "Demuxer and parser should allow for reading of" do
     @tag :tmp_dir
-    test "a single H264 track", %{tmp_dir: dir} do
-      in_path = "test/fixtures/in_video.h264"
-      mp4_path = Path.join(dir, "out.mp4")
-      out_path = Path.join(dir, "out_video.h264")
-
-      muxing_spec = [
-        child(:file, %Membrane.File.Source{location: in_path})
-        |> child(:parser, %Membrane.H264.Parser{
-          generate_best_effort_timestamps: %{framerate: {30, 1}},
-          output_stream_structure: :avc1
-        })
-        |> child(:muxer, %Membrane.MP4.Muxer.ISOM{
-          chunk_duration: Membrane.Time.seconds(1),
-          fast_start: true
-        })
-        |> child(:sink, %Membrane.File.Sink{location: mp4_path})
-      ]
-
-      Pipeline.start_link_supervised!(structure: muxing_spec) |> wait_for_pipeline_termination()
-
-      demuxing_spec = [
-        child(:file, %Membrane.File.Source{location: mp4_path})
-        |> child(:demuxer, Membrane.MP4.Demuxer.ISOM)
-        |> via_out(Pad.ref(:output, 1))
-        |> child(:parser, %Membrane.H264.Parser{output_stream_structure: :annexb})
-        |> child(:sink, %Membrane.File.Sink{location: out_path})
-      ]
-
-      Pipeline.start_link_supervised!(structure: demuxing_spec) |> wait_for_pipeline_termination()
-
-      assert_files_equal(out_path, in_path)
+    test "a single fast start H264 track", %{tmp_dir: dir} do
+      perform_test([:H264], true, dir)
     end
 
     @tag :tmp_dir
-    test "a single AAC track", %{tmp_dir: dir} do
-      in_path = "test/fixtures/in_audio.aac"
-      mp4_path = Path.join(dir, "out.mp4")
-      out_path = Path.join(dir, "out_audio.aac")
-
-      muxing_spec = [
-        child(:file, %Membrane.File.Source{location: in_path})
-        |> child(:parser, %Membrane.AAC.Parser{
-          out_encapsulation: :none,
-          output_config: :esds
-        })
-        |> child(:muxer, %Membrane.MP4.Muxer.ISOM{
-          chunk_duration: Membrane.Time.seconds(1),
-          fast_start: true
-        })
-        |> child(:sink, %Membrane.File.Sink{location: mp4_path})
-      ]
-
-      Pipeline.start_link_supervised!(structure: muxing_spec) |> wait_for_pipeline_termination()
-
-      demuxing_spec = [
-        child(:file, %Membrane.File.Source{location: mp4_path})
-        |> child(:demuxer, Membrane.MP4.Demuxer.ISOM)
-        |> via_out(Pad.ref(:output, 1))
-        |> child(:parser, %Membrane.AAC.Parser{
-          out_encapsulation: :ADTS
-        })
-        |> child(:sink, %Membrane.File.Sink{location: out_path})
-      ]
-
-      Pipeline.start_link_supervised!(structure: demuxing_spec) |> wait_for_pipeline_termination()
-
-      assert_files_equal(out_path, in_path)
+    test "a single non-fast-start H264 track", %{tmp_dir: dir} do
+      perform_test([:H264], false, dir)
     end
 
     @tag :tmp_dir
-    test "H264 and AAC tracks", %{tmp_dir: dir} do
-      in_video_path = "test/fixtures/in_video.h264"
-      in_audio_path = "test/fixtures/in_audio.aac"
-
-      mp4_path = Path.join(dir, "out.mp4")
-
-      out_video_path = Path.join(dir, "out_video.h264")
-      out_audio_path = Path.join(dir, "out_audio.aac")
-
-      muxing_spec = [
-        child(:file_video, %Membrane.File.Source{location: in_video_path})
-        |> child(:video_parser, %Membrane.H264.Parser{
-          generate_best_effort_timestamps: %{framerate: {30, 1}},
-          output_stream_structure: :avc1
-        })
-        |> child(:muxer, %Membrane.MP4.Muxer.ISOM{
-          chunk_duration: Membrane.Time.seconds(1),
-          fast_start: true
-        })
-        |> child(:sink, %Membrane.File.Sink{location: mp4_path}),
-        child(:file_audio, %Membrane.File.Source{location: in_audio_path})
-        |> child(:audio_parser, %Membrane.AAC.Parser{
-          out_encapsulation: :none,
-          output_config: :esds
-        })
-        |> get_child(:muxer)
-      ]
-
-      Pipeline.start_link_supervised!(structure: muxing_spec) |> wait_for_pipeline_termination()
-
-      demuxing_spec = [
-        child(:file, %Membrane.File.Source{location: mp4_path})
-        |> child(:demuxer, Membrane.MP4.Demuxer.ISOM)
-        |> via_out(Pad.ref(:output, 1))
-        |> child(:parser_video, %Membrane.H264.Parser{output_stream_structure: :annexb})
-        |> child(:sink_video, %Membrane.File.Sink{location: out_video_path}),
-        get_child(:demuxer)
-        |> via_out(Pad.ref(:output, 2))
-        |> child(:audio_parser, %Membrane.AAC.Parser{
-          out_encapsulation: :ADTS
-        })
-        |> child(:sink_audio, %Membrane.File.Sink{location: out_audio_path})
-      ]
-
-      Pipeline.start_link_supervised!(structure: demuxing_spec)
-      |> wait_for_pipeline_termination([:sink_audio, :sink_video])
-
-      assert_files_equal(out_video_path, in_video_path)
-      assert_files_equal(out_audio_path, in_audio_path)
+    test "a single fast start AAC track", %{tmp_dir: dir} do
+      perform_test([:AAC], true, dir)
+    end
+
+    @tag :tmp_dir
+    test "a single non-fast-start AAC track", %{tmp_dir: dir} do
+      perform_test([:AAC], false, dir)
+    end
+
+    @tag :tmp_dir
+    test "fast start H264 and AAC tracks", %{tmp_dir: dir} do
+      perform_test([:H264, :AAC], true, dir)
+    end
+
+    @tag :tmp_dir
+    test "non-fast-start H264 and AAC tracks", %{tmp_dir: dir} do
+      perform_test([:H264, :AAC], false, dir)
     end
   end
 
@@ -157,6 +65,127 @@ defmodule Membrane.MP4.Demuxer.ISOM.IntegrationTest do
     assert demuxing_buffers == ref_buffers
   end
 
+  defp perform_test(tracks, fast_start, dir)
+
+  defp perform_test([:H264], fast_start, dir) do
+    in_path = "test/fixtures/in_video.h264"
+    mp4_path = Path.join(dir, "out.mp4")
+    out_path = Path.join(dir, "out_video.h264")
+
+    muxing_spec = [
+      child(:file, %Membrane.File.Source{location: in_path})
+      |> child(:parser, %Membrane.H264.Parser{
+        generate_best_effort_timestamps: %{framerate: {30, 1}},
+        output_stream_structure: :avc1
+      })
+      |> child(:muxer, %Membrane.MP4.Muxer.ISOM{
+        chunk_duration: Membrane.Time.seconds(1),
+        fast_start: fast_start
+      })
+      |> child(:sink, %Membrane.File.Sink{location: mp4_path})
+    ]
+
+    Pipeline.start_link_supervised!(structure: muxing_spec) |> wait_for_pipeline_termination()
+
+    demuxing_spec = [
+      child(:file, %Membrane.File.Source{location: mp4_path})
+      |> child(:demuxer, Membrane.MP4.Demuxer.ISOM)
+      |> via_out(Pad.ref(:output, 1))
+      |> child(:parser, %Membrane.H264.Parser{output_stream_structure: :annexb})
+      |> child(:sink, %Membrane.File.Sink{location: out_path})
+    ]
+
+    Pipeline.start_link_supervised!(structure: demuxing_spec) |> wait_for_pipeline_termination()
+
+    assert_files_equal(out_path, in_path)
+  end
+
+  defp perform_test([:AAC], fast_start, dir) do
+    in_path = "test/fixtures/in_audio.aac"
+    mp4_path = Path.join(dir, "out.mp4")
+    out_path = Path.join(dir, "out_audio.aac")
+
+    muxing_spec = [
+      child(:file, %Membrane.File.Source{location: in_path})
+      |> child(:parser, %Membrane.AAC.Parser{
+        out_encapsulation: :none,
+        output_config: :esds
+      })
+      |> child(:muxer, %Membrane.MP4.Muxer.ISOM{
+        chunk_duration: Membrane.Time.seconds(1),
+        fast_start: fast_start
+      })
+      |> child(:sink, %Membrane.File.Sink{location: mp4_path})
+    ]
+
+    Pipeline.start_link_supervised!(structure: muxing_spec) |> wait_for_pipeline_termination()
+
+    demuxing_spec = [
+      child(:file, %Membrane.File.Source{location: mp4_path})
+      |> child(:demuxer, Membrane.MP4.Demuxer.ISOM)
+      |> via_out(Pad.ref(:output, 1))
+      |> child(:parser, %Membrane.AAC.Parser{
+        out_encapsulation: :ADTS
+      })
+      |> child(:sink, %Membrane.File.Sink{location: out_path})
+    ]
+
+    Pipeline.start_link_supervised!(structure: demuxing_spec) |> wait_for_pipeline_termination()
+
+    assert_files_equal(out_path, in_path)
+  end
+
+  defp perform_test([:H264, :AAC], fast_start, dir) do
+    in_video_path = "test/fixtures/in_video.h264"
+    in_audio_path = "test/fixtures/in_audio.aac"
+
+    mp4_path = Path.join(dir, "out.mp4")
+
+    out_video_path = Path.join(dir, "out_video.h264")
+    out_audio_path = Path.join(dir, "out_audio.aac")
+
+    muxing_spec = [
+      child(:file_video, %Membrane.File.Source{location: in_video_path})
+      |> child(:video_parser, %Membrane.H264.Parser{
+        generate_best_effort_timestamps: %{framerate: {30, 1}},
+        output_stream_structure: :avc1
+      })
+      |> child(:muxer, %Membrane.MP4.Muxer.ISOM{
+        chunk_duration: Membrane.Time.seconds(1),
+        fast_start: fast_start
+      })
+      |> child(:sink, %Membrane.File.Sink{location: mp4_path}),
+      child(:file_audio, %Membrane.File.Source{location: in_audio_path})
+      |> child(:audio_parser, %Membrane.AAC.Parser{
+        out_encapsulation: :none,
+        output_config: :esds
+      })
+      |> get_child(:muxer)
+    ]
+
+    Pipeline.start_link_supervised!(structure: muxing_spec) |> wait_for_pipeline_termination()
+
+    demuxing_spec = [
+      child(:file, %Membrane.File.Source{location: mp4_path})
+      |> child(:demuxer, Membrane.MP4.Demuxer.ISOM)
+      |> via_out(Pad.ref(:output, 1))
+      |> child(:parser_video, %Membrane.H264.Parser{output_stream_structure: :annexb})
+      |> child(:sink_video, %Membrane.File.Sink{location: out_video_path}),
+      get_child(:demuxer)
+      |> via_out(Pad.ref(:output, 2))
+      |> child(:audio_parser, %Membrane.AAC.Parser{
+        out_encapsulation: :ADTS
+      })
+      |> child(:sink_audio, %Membrane.File.Sink{location: out_audio_path})
+    ]
+
+    Pipeline.start_link_supervised!(structure: demuxing_spec)
+    |> wait_for_pipeline_termination([:sink_audio, :sink_video])
+
+    assert_files_equal(out_video_path, in_video_path)
+    assert_files_equal(out_audio_path, in_audio_path)
+  end
+
   defp flush_buffers(acc \\ []) do
     receive do
       {Membrane.Testing.Pipeline, _pid, {:handle_child_notification, {{:buffer, buffer}, :sink}}} ->