[Bug 642671] fieldanalysis: New element for analysing video input to produce progressive output

[GStreamer (bugzilla.gnome.org)]

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

Sebastian Dröge <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #181214|none                        |needs-work
             status|                            |

--- Comment #10 from Sebastian Dröge <slomo at circular-chaos.org> 2011-02-24 13:24:47 UTC ---
Review of attachment 181214:
 --> (https://bugzilla.gnome.org/review?bug=642671&attachment=181214)

Looks good in general (of course I didn't review the details of the algorithms)
but please don't use // comments and some other comments below. After fixing
these feel free to push this to -bad :)

::: gst/fieldanalysis/gstfieldanalysis.c
@@ +333,3 @@
+  gst_element_add_pad (GST_ELEMENT (filter), filter->srcpad);
+
+  filter->frames = g_queue_new ();

You're leaking this. First of all the queue should probably be cleared on caps
changes and when going back to READY and then it should be freed in
GObject::finalize(). And flush the queue on FLUSH_START events and maybe
NEWSEGMENT events with update=false. Note that the queue does not automatically
unref the buffers if you clear it.

@@ +550,3 @@
+  GstCaps *caps;
+
+  caps = gst_caps_copy (GST_PAD_CAPS (filter->srcpad));

Maybe add a fast-path here if the srcpad caps are not to be changed. Then you
don't need to allocate new caps and do all the logic below but only need to set
buffer flags. This code here sets new caps on every single buffer, which then
requires expensive deep caps-equality checks downstream instead of just
comparing the pointers.

@@ +871,3 @@
+
+  comb_mask = (guint8 *) g_malloc (width);
+  block_scores = (guint *) g_malloc0 ((width / block_width) * sizeof (guint));

Maybe allocate this memory a single time in set_caps instead of allocating it
for every single frame. Same goes for the other detect functions below

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