Don't fail when unknown sample types are encountered (#116)

* Ignore unknown sample types * Filter out nil sample descriptions * Put the filtered out sample tables into a map * Make sure the stsz entries list is only parsed when sample size equals 0 (as the specification defines. Ignore unsupported sample types. Add tests checking if unsupported sample types are ignored * Implement reviewers suggestions * Update lib/membrane_mp4/demuxer/isom.ex Co-authored-by: Mateusz Front <[email protected]> --------- Co-authored-by: Mateusz Front <[email protected]>
membraneframework · Aug 22, 2024 · 235566f · 235566f
1 parent df35516
commit 235566f
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 7 deletions.
diff --git a/lib/membrane_mp4/container/schema.ex b/lib/membrane_mp4/container/schema.ex
@@ -315,11 +315,13 @@ defmodule Membrane.MP4.Container.Schema do
                               [
                                 sample_size: :uint32,
                                 sample_count: :uint32,
-                                entry_list:
+                                entry_list: {
                                   {:list,
                                    [
                                      entry_size: :uint32
-                                   ]}
+                                   ]},
+                                  when: {:sample_size, value: 0}
+                                }
                               ]
                         ],
                         stco: [

diff --git a/lib/membrane_mp4/demuxer/isom.ex b/lib/membrane_mp4/demuxer/isom.ex
@@ -430,7 +430,9 @@ defmodule Membrane.MP4.Demuxer.ISOM do
   end
 
   defp match_tracks_with_pads(ctx, state) do
-    sample_tables = state.samples_info.sample_tables
+    sample_tables =
+      state.samples_info.sample_tables
+      |> reject_unsupported_sample_types()
 
     output_pads_data =
       ctx.pads
@@ -464,6 +466,7 @@ defmodule Membrane.MP4.Demuxer.ISOM do
 
           kind_to_tracks =
             sample_tables
+            |> reject_unsupported_sample_types()
             |> Enum.group_by(
               fn {_track_id, table} -> sample_description_to_kind(table.sample_description) end,
               fn {track_id, _table} -> track_id end
@@ -500,6 +503,7 @@ defmodule Membrane.MP4.Demuxer.ISOM do
 
     tracks_codecs =
       state.samples_info.sample_tables
+      |> reject_unsupported_sample_types()
       |> Enum.map(fn {_track, table} -> table.sample_description.__struct__ end)
 
     raise """
@@ -518,6 +522,7 @@ defmodule Membrane.MP4.Demuxer.ISOM do
   defp maybe_get_track_notifications(%{pads_linked_before_notification?: false} = state) do
     new_tracks =
       state.samples_info.sample_tables
+      |> reject_unsupported_sample_types()
       |> Enum.map(fn {track_id, table} ->
         pad_id = state.track_to_pad_id[track_id]
         {pad_id, table.sample_description}
@@ -528,6 +533,7 @@ defmodule Membrane.MP4.Demuxer.ISOM do
 
   defp get_stream_format(state) do
     state.samples_info.sample_tables
+    |> reject_unsupported_sample_types()
     |> Enum.map(fn {track_id, table} ->
       pad_id = state.track_to_pad_id[track_id]
       {:stream_format, {Pad.ref(:output, pad_id), table.sample_description}}
@@ -639,7 +645,12 @@ defmodule Membrane.MP4.Demuxer.ISOM do
   defp all_pads_connected?(_ctx, %{samples_info: nil}), do: false
 
   defp all_pads_connected?(ctx, state) do
-    tracks = 1..state.samples_info.tracks_number
+    count_of_supported_tracks =
+      state.samples_info.sample_tables
+      |> reject_unsupported_sample_types()
+      |> Enum.count()
+
+    tracks = 1..count_of_supported_tracks
 
     pads =
       ctx.pads
@@ -653,14 +664,19 @@ defmodule Membrane.MP4.Demuxer.ISOM do
 
   defp flush_samples(state) do
     actions =
-      Enum.map(state.buffered_samples, fn {track_id, track_samples} ->
+      Enum.flat_map(state.buffered_samples, fn {track_id, track_samples} ->
         buffers =
           track_samples
           |> Enum.reverse()
           |> Enum.map(fn {buffer, ^track_id} -> buffer end)
 
         pad_id = state.track_to_pad_id[track_id]
-        {:buffer, {Pad.ref(:output, pad_id), buffers}}
+
+        if pad_id != nil do
+          [buffer: {Pad.ref(:output, pad_id), buffers}]
+        else
+          []
+        end
       end)
 
     state = %{state | buffered_samples: %{}}
@@ -685,4 +701,8 @@ defmodule Membrane.MP4.Demuxer.ISOM do
   defp get_mdat_header_beginning([{_other_name, box} | rest]) do
     box.header_size + box.size + get_mdat_header_beginning(rest)
   end
+
+  defp reject_unsupported_sample_types(sample_tables) do
+    Map.reject(sample_tables, fn {_track_id, table} -> table.sample_description == nil end)
+  end
 end
diff --git a/lib/membrane_mp4/movie_box/sample_table_box.ex b/lib/membrane_mp4/movie_box/sample_table_box.ex
@@ -2,6 +2,7 @@ defmodule Membrane.MP4.MovieBox.SampleTableBox do
   @moduledoc false
 
   require Membrane.H264
+  require Membrane.Logger
   alias Membrane.MP4.{Container, Helper, Track.SampleTable}
   alias Membrane.{AAC, H264, H265, Opus}
 
@@ -292,6 +293,10 @@ defmodule Membrane.MP4.MovieBox.SampleTableBox do
     offsets |> Enum.map(fn %{chunk_offset: offset} -> offset end)
   end
 
+  defp unpack_sample_sizes(%{fields: %{entry_list: [], sample_count: 1, sample_size: sample_size}}) do
+    [sample_size]
+  end
+
   defp unpack_sample_sizes(%{fields: %{entry_list: sizes}}) do
     sizes |> Enum.map(fn %{entry_size: size} -> size end)
   end
@@ -327,4 +332,9 @@ defmodule Membrane.MP4.MovieBox.SampleTableBox do
   defp unpack_sample_description(%{children: [{:Opus, %{children: boxes}}]}) do
     %Opus{channels: boxes[:dOps].fields.output_channel_count, self_delimiting?: false}
   end
+
+  defp unpack_sample_description(%{children: [{sample_type, _sample_metadata}]}) do
+    Membrane.Logger.warning("Unknown sample type: #{inspect(sample_type)}")
+    nil
+  end
 end
diff --git a/test/fixtures/isom/ref_video_with_tmcd.mp4 b/test/fixtures/isom/ref_video_with_tmcd.mp4
diff --git a/test/membrane_mp4/demuxer/isom/demuxer_test.exs b/test/membrane_mp4/demuxer/isom/demuxer_test.exs
@@ -242,7 +242,40 @@ defmodule Membrane.MP4.Demuxer.ISOM.DemuxerTest do
     @tag :tmp_dir
     test "output pad connected after moov box has been read", %{tmp_dir: dir} do
       out_path = Path.join(dir, "out")
-      filename = "test/fixtures/isom/ref_video_fast_start.mp4"
+      filename = "test/fixtures/isom/ref_video.mp4"
+
+      pipeline =
+        start_remote_pipeline!(
+          filename: filename,
+          file_source_chunk_size: File.stat!(filename).size - 1
+        )
+
+      assert_receive %RCMessage.Notification{
+                       element: :demuxer,
+                       data: {:new_tracks, [{1, _payload}]},
+                       from: _
+                     },
+                     2000
+
+      structure = [
+        get_child(:demuxer)
+        |> via_out(Pad.ref(:output, 1))
+        |> child(:sink, %Membrane.File.Sink{location: out_path})
+      ]
+
+      RCPipeline.exec_actions(pipeline, spec: {structure, []})
+      assert_receive %RCMessage.EndOfStream{element: :demuxer, pad: :input}, 2000
+      assert_receive %RCMessage.EndOfStream{element: :sink, pad: :input}, 2000
+
+      RCPipeline.terminate(pipeline)
+
+      assert_files_equal(out_path, ref_path_for("video"))
+    end
+
+    @tag :tmp_dir
+    test "file is properly demuxed when unsupported sample type is present", %{tmp_dir: dir} do
+      out_path = Path.join(dir, "out")
+      filename = "test/fixtures/isom/ref_video_with_tmcd.mp4"
 
       pipeline =
         start_remote_pipeline!(