[gstreamer-bugs] [Bug 626181] [pbutils] Add enhanced gstfactorylists

[GStreamer (bugzilla.gnome.org)]

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

--- Comment #8 from Sebastian Dröge <slomo at circular-chaos.org> 2010-08-06 16:23:04 UTC ---
Review of attachment 167259:
 --> (https://bugzilla.gnome.org/review?bug=626181&attachment=167259)

First of all I don't think this belongs into core... There's stuff about audio
and video in here.

Also, why don't you use a GstCaps instead of a GValueArray? That makes things a
bit easier I guess... especially because your caps inside the value array only
have a single, simple structure ;)

And then you might want to try to port subtitleoverlay to this. It needs a list
of subtitle parsers/decoders and subtitle renderers.

Other comments below

::: gst-libs/gst/pbutils/gstfactorylists.c
@@ +348,3 @@
+    while (!gst_structure_foreach (st,
+            (GstStructureForeachFunc) remove_range_foreach, st));
+  }

For this you might want to take a look at
http://svn.debian.org/viewsvn/pkg-gstreamer/unstable/gstreamer0.10/debian/gst-codec-info.c?revision=3298&view=markup

Search for "codec_data" and remove_min_max_fields(). I think that's a bit safer
and better than simply removing *all* ranges.

@@ +353,3 @@
+
+  /* Explode to individual structures */
+  res2 = gst_caps_normalize (res);

Why is the normalizing here necessary?

@@ +376,3 @@
+    tmpc = gst_caps_new_full (st, NULL);
+
+    /* Check if the caps are already present */

If you were using a GstCaps instead of a GValueArray of normalized, simple caps
this could be done by gst_caps_merge_structure() ;)

::: gst-libs/gst/pbutils/gstfactorylists.h
@@ +40,3 @@
+ * @GST_FACTORY_LIST_MEDIA_VIDEO: Elements handling video media types
+ * @GST_FACTORY_LIST_MEDIA_AUDIO: Elements handling audio media types
+ * @GST_FACTORY_LIST_MEDIA_IMAGE: Elements handling image media types

Maybe add something for subtitles, subpictures and metadata here

@@ +61,3 @@
+  GST_FACTORY_LIST_DEPAYLOADER  = (1 << 8),
+
+  GST_FACTORY_LIST_MAX_ELEMENTS = (1 << 16),

If you want to add special purpose elements here 16 might be too small

@@ +101,3 @@
+GValueArray * gst_factory_list_get_elements (GstFactoryListType type, GstRank
minrank);
+
+void          gst_factory_list_debug        (GValueArray *array);

Should this debug function really be public?

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