[gstreamer-bugs] [Bug 604869] Add timeout in freeze element

[GStreamer (bugzilla.gnome.org)]

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

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

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

--- Comment #1 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2009-12-18 09:01:56 UTC ---
Review of attachment 149949:
 --> (https://bugzilla.gnome.org/review?bug=604869&attachment=149949)

::: gst/freeze/gstfreeze.c
@@ +45,3 @@
   ARG_0,
   ARG_MAX_BUFFERS,
+  ARG_TIMEOUT,

No trailing comma for the last enum please (some compilers don't like that)

@@ +55,3 @@
     "Gergely Nagy <gergely.nagy at neteyes.hu>,"
+    "Renato Filho <renato.filho at indt.org.br>,"
+    "Daniel Diaz <yosoy at danieldiaz.org>");

Please don't add your name here for trivial patches, feel free to add a
Copyright line though.

@@ +126,3 @@
+      g_param_spec_int ("timeout",
+          "timeout",
+          "Timeout before closing stream", 0, G_MAXINT, 1,
G_PARAM_READWRITE));

You should mention the units of the timeout here and that 0 = no timeout.

@@ +403,3 @@
+
+  gst_element_get_state (GST_ELEMENT (freeze), &cur_state, NULL, 0);
+  if (cur_state != GST_STATE_PLAYING)

This does not really seem right conceptually. A filter should behave the same
in paused and playing state really. Also, you don't take account of
intermediate pause states and the like (not a big deal, just saying).

@@ +406,3 @@
+    return TRUE;
+
+  gst_pad_push_event (freeze->srcpad, gst_event_new_eos ());

This is not really entirely correct. At the very least the EOS should be sent
from the streaming thread.

@@ +416,3 @@
+
+  if (freeze->timeout > 0)
+    g_timeout_add (freeze->timeout * 1000, gst_freeze_finish_stream, freeze);

You can't/shouldn't rely on the default GLib main context being iterated in an
element.

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