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

[GStreamer (GNOME Bugzilla)]

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

--- Comment #43 from Reynaldo H. Verdejo Pinochet <reynaldo at osg.samsung.com> ---
(In reply to Seungha Yang from comment #41)
> (In reply to Reynaldo H. Verdejo Pinochet from comment #34)
> > Review of attachment 352561 [details] [review] [review]:
> 
> > ::: 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?
> Removed fixme and added code for checking exact required size.
> 


Thank you!

> 
> > @@ +323,3 @@
> > +    duration = ~duration;
> > +    absolute_time = ~time;
> > +    GST_ERROR ("Error getting box's flags field");
> > 
> 
> Couldn't find any API for both getting and ~= operation at once from
> gstbytereader.c...


Can you elaborate on why are you applying bitwise-NOT operations _twice_ on
time & duration before assigning the resulting values to the absolute_ vars?

> 
> 
> > @@ +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)
> Done 
> 
> > 
> > @@ +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.
> 
> Should we get rid of them from this function?


If you want.

Thanks again for staying on top of this.

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