[Bug 793333] matroskademux: Allow Matroska headers to be read more than once

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=793333

--- Comment #19 from Alicia Boya García <aboya at igalia.com> ---
Review of attachment 373731:
 --> (https://bugzilla.gnome.org/review?bug=793333&attachment=373731)

::: gst/matroska/matroska-demux.c
@@ +651,3 @@
           GST_ERROR_OBJECT (demux, "Invalid TrackNumber 0");
           ret = GST_FLOW_ERROR;
           break;

Note this is gst_matroska_demux_parse_stream() [The review tool includes a
header for the function name in the old version but not in the new one]

After the refactor gst_matroska_demux_parse_stream() is supposed to be
reasonably stateless and depend as little as possible on the bigger
matroskademux state. Therefore, it makes no sense for it to reject tracks on
the basis of being it the first time they appear or not.

This will be very important in the next patch, where we will want the parsed
track to have an existing TrackNumber. It makes much more sense to check that
outside.

@@ +1514,3 @@
+      break;
+
+  g_assert (demux->common.src->len == demux->common.num_streams);

This was present in the original version. I guess the intention is to
explicitly list all that is not supported, which is fine for me.

@@ +3112,3 @@
+          if (gst_matroska_read_common_tracknumber_unique (&demux->common,
+                  track->num))
+            gst_matroska_demux_add_stream (demux, track);

With pleasure, I don't like ifs without braces, they are dangerous.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.