[gstreamer-bugs] [Bug 623063] [jpegdec] add "max-errors" property

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=623063
  GStreamer | gst-plugins-good | 0.10.22

Tim-Philipp Müller <t.i.m> changed:

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

--- Comment #4 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2010-08-09 22:35:14 UTC ---
Review of attachment 167466:
 --> (https://bugzilla.gnome.org/review?bug=623063&attachment=167466)

::: ext/jpeg/gstjpegdec.h
@@ +117,3 @@
+ 
+  /* number of errors we're currently at before reporting GST_FLOW_ERROR */
+  gint     max_error_count;

Should be error_count IMHO.

::: ext/jpeg/gstjpegdec.c
@@ +54,2 @@
 #define JPEG_DEFAULT_IDCT_METHOD    JDCT_FASTEST
+#define JPEG_DEFAULT_MAX_ERRORS    -1

What does -1 mean here? (vs. 0)

@@ +195,3 @@
+ 
+  g_object_class_install_property (gobject_class, PROP_MAX_ERRORS,
+      g_param_spec_int ("error-after", "Error After", "Error after N buffers",

You forgot to changes this to "max-errors" here. (Description needs fixing too
then btw.)

@@ +196,3 @@
+  g_object_class_install_property (gobject_class, PROP_MAX_ERRORS,
+      g_param_spec_int ("error-after", "Error After", "Error after N buffers",
+          G_MININT, G_MAXINT, JPEG_DEFAULT_MAX_ERRORS,

Surely the minimum is 0? Or -1 for 'default' (which could be mode-dependent,
ie. 0 for still image and N for MJPEG).

@@ +1681,3 @@
       break;
+    case PROP_MAX_ERRORS:
+      dec->max_errors = g_value_get_int (value);

Technically this needs locking and/or g_atomic_int_{get|set}(), but then the
method property dosn't do that either..

@@ +1722,3 @@
   switch (transition) {
     case GST_STATE_CHANGE_READY_TO_PAUSED:
+      dec->max_error_count = dec->max_errors;

Why this? Shouldn't it be reset to 0?

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