[Bug 754864] qtdemux:check multi trex to find track id in mp4 (DASH) stream.

[GStreamer (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #311110|none                        |needs-work
             status|                            |

--- Comment #2 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 311110:
 --> (https://bugzilla.gnome.org/review?bug=754864&attachment=311110)

Thanks for the patch!

::: gst/isomp4/qtdemux.c
@@ +80,2 @@
 #ifdef HAVE_ZLIB
+#include <zlib.h>

Please don't include irrelevant changes in the same patch

@@ +613,3 @@
   qtdemux->flowcombiner = gst_flow_combiner_new ();
+  memset (qtdemux->track_fragment_id, 0,
+  sizeof (guint32) * GST_QTDEMUX_MAX_STREAMS);

memset (qtdemux->track_fragment_id, 0, sizeof (qtdemux->track_fragment_id));

That way this code doesn't have to be changed if the array is ever changed

@@ +4350,3 @@

+  if (G_UNLIKELY (QTSEGMENT_IS_EMPTY (&stream->
+              segments[stream->segment_index]))) {

No indentation changes in the same patch please :)

@@ +5102,3 @@
   /* If we're doing a keyframe-only trickmode, only push keyframes on video
streams */
+  if (G_UNLIKELY (qtdemux->segment.
+          flags & GST_SEGMENT_FLAG_TRICKMODE_KEY_UNITS)) {

No indentation changes of unrelated code in the same patch please :)

@@ +8455,3 @@
       goto skip_track;
     }
+  if (qtdemux_check_track_fragment (qtdemux, track_id)) {

qtdemux_check_track_is_in_trex() or similar seems more descriptive here

If I'm not mistaken you can also just use qtdemux_parse_trex() here instead,
which is called many lines below in this function... instead of adding and
calling the new get_track_fragment_id()

@@ +8460,3 @@
+  } else {
+    GST_WARNING_OBJECT (qtdemux, "skip track id %u, it's not
fragmented",track_id);
+    return TRUE;

There's a "skip_track" goto label for this here

@@ +11645,3 @@
   }
+  memset (qtdemux->track_fragment_id, 0,sizeof (guint32) *
GST_QTDEMUX_MAX_STREAMS);
+  qtdemux_get_track_fragment_id (qtdemux, mvex);

You don't necessarily have mvex here, see the "if (mvex)" a few lines above. It
should probably be moved in that if block

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