[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
----------------------------------------------------------------------------
 Attachment #164206|none                        |needs-work
             status|                            |

--- Comment #1 from Edward Hervey <bilboed at gmail.com> 2010-06-21 14:07:17 UTC ---
Review of attachment 164206:
 --> (https://bugzilla.gnome.org/review?bug=622276&attachment=164206)

::: gst/h263parse/h263parse.c
@@ +376,3 @@
+gst_h263_parse_get_params (GstH263Parse * h263parse, H263Params * params,
+    gboolean fast)
+{

it would be a bit cleaner if this was in a separate file ? Along with all the
parsing-specific structures. Then it would leave only the gst-specific parts in
gsth263parse.[ch]

@@ +808,3 @@
+
+static gint
+gst_h263_parse_get_profile (GstH263Parse * h263parse)

Same thing, this could be moved to a separate file and just take a H263Params
structure as input.

@@ +930,3 @@
+
+static gint
+gst_h263_parse_get_level (GstH263Parse * h263parse)

Same comment as for get_profile

@@ +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.

::: 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

-- 
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.