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

GStreamer (GNOME Bugzilla) bugzilla at gnome.org
Sun Jun 4 05:07:34 UTC 2017


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

--- Comment #37 from Seungha Yang <sh.yang at lge.com> ---
(In reply to Reynaldo H. Verdejo Pinochet from comment #33)
> Review of attachment 352560 [details] [review]:

Always thanks for your detailed review :) 

I'd like to ask and answer about your suggestion

* About version handling,
I prefer to use if/else instead of switch because
- ISO/IEC 14496-12:2015 is defining like if/else and qtdemux also did it.
So I'd like to make them similar. In my opinion, defining more version is not
probable in near future.

* About $had_.. variables
My intention was to check existence of target child boxes on that boxes.
For example, in case of gst_isoff_mdia_box_parse() which is for parsing mdia
box, both mdhd and hdlr are mandatory (by spec, and for my use case) child
boxes. So I defined two $had_ variables to check them. If one of them are
omitted, it's error cases. However, since mdia box might have other unknown
boxes as its child box (it's not error), we should iterate all bytes to search
them.

In this context, could you please inform more detail about following comment?
> There's no need for two $had_ variables. Use only one and consider making the > function return the found value instead of a boolean if you think it's useful.
> ...
> At a first glance it seems like you can drop all the had_track variable
> logic if you simple "goto error;" before breaking.

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


More information about the gstreamer-bugs mailing list