[gstreamer-bugs] [Bug 622276] Add an h263parse element

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=622276
  GStreamer | gst-plugins-bad | git

Edward Hervey <bilboed> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |bilboed at gmail.com
     Ever Confirmed|0                           |1

--- Comment #3 from Edward Hervey <bilboed at gmail.com> 2010-07-07 09:58:37 UTC ---
(In reply to comment #2)
> > @@ +930,3 @@
> > +
> > +static gint
> > +gst_h263_parse_get_level (GstH263Parse * h263parse)
> > 
> > Same comment as for get_profile
> 
> Can move to a separate file. get_level needs to take a GstH263Parse* since the
> bitrate isn't part of params (and IMHO should not be, since it basically
> represents stream params parsed out of the header). I let get_profile also take
> the same parameter for consistency, but I'm open to changing this.

  If you only need one extra parameter, can't you just add it as a return
argument ?

> 
> 
> > @@ +1081,3 @@
> > +    if (h263parse->profile != -1)
> > +      gst_caps_set_simple (caps, "profile", G_TYPE_STRING,
> > +          profiles[h263parse->profile], NULL);
> > 
> > If profile is only going to be a numerical value, let's keep it as an integer.
> > 
> > @@ +1086,3 @@
> > +    if (h263parse->level != -1) {
> > +      level_idx = (h263parse->level / 10) + (h263parse->level % 10) - 1;
> > +      gst_caps_set_simple (caps, "level", G_TYPE_STRING, levels[level_idx],
> > 
> > Same comment as for profile.
> 
> Since some codecs have non-numeric profiles/levels, I figured we should just
> keep them all as strings instead of having to worry about which codec has a
> string profile vs. which has an integer profile. Again, I'm not too religious
> about this, so if the common feeling is to go with integers here, I'll change
> it.

  If for this media type we only have numerical values, let's stick with
integers, makes the parsing code easier to handle (and therefore less
bug-prone).

> 
> 
> > ::: gst/h263parse/h263parse.h
> > @@ +67,3 @@
> > +  H263_OPTION_DPS_MODE  = 1 << 14
> > +} H263OptionalFeatures;
> > +
> > 
> > Since this is a plugin, there's maybe no need to have this flag definition here
> > 
> > @@ +104,3 @@
> > +  UUI_IS_01,
> > +} H263UUI;
> > +
> > 
> > Same comment for all structures above.
> > 
> > @@ +128,3 @@
> > +  H263ParseState        state;
> > +} H263Params;
> > +
> > 
> > These too
> 
> Since the header is never installed, will this make a difference?

  Just cleaner code :)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.