[Bug 777825] isoff: Move isoff to gst-libs

[GStreamer (GNOME Bugzilla)]

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

Reynaldo H. Verdejo Pinochet <reynaldo at osg.samsung.com> changed:

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

--- Comment #34 from Reynaldo H. Verdejo Pinochet <reynaldo at osg.samsung.com> ---
Review of attachment 352561:
 --> (https://bugzilla.gnome.org/review?bug=777825&attachment=352561)

Nothing severe but worth taking a look ...

::: gst-libs/gst/isoff/gstisoff.c
@@ +297,3 @@
+  memset (tfxd, 0, sizeof (*tfxd));
+
+  guint64 absolute_time = 0;

How much would it take to get rid of the fixme?

@@ +323,3 @@
+    duration = ~duration;
+    absolute_time = ~time;
+    GST_ERROR ("Error getting box's flags field");

assignment block can be replaced by

byte_reader_get(...);
byte_reader_get(...);
absolute_time = time;
absolute_duration = duration;

and then the following two (while correct) also become unnecessary:

time = ~time;
duration = ~duration;

or maybe I'm missing something?

@@ +329,3 @@
+  tfxd->duration = absolute_duration;
+
+

The _LOG() call here is redundant considering the info provided can be derived
from the return value (The function only returns TRUE if the box has been
parsed)

@@ +351,3 @@
+    GST_ERROR ("Error getting box's flags field");
+    goto error;
+    gst_byte_reader_get_uint32_be (reader, &duration);

seems like factoring out the two above checks from this function and
_tfxd_box_parse() wouldn't hurt. Make it an inline func if you do.

@@ +383,3 @@
+      duration = ~duration;
+      absolute_time = ~time;
+{

Please take a look at the above comment for the similar code in
gst_isoff_tfxd_box_parse()

@@ +391,3 @@
+  }
+
+  guint8 index = 0;

DITTO, redundant log message.

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