After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 604869 - Add timeout in freeze element
Add timeout in freeze element
Status: RESOLVED INCOMPLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-12-17 23:46 UTC by Daniel Díaz
Modified: 2011-09-22 09:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add timeout property to freeze (4.14 KB, patch)
2009-12-17 23:46 UTC, Daniel Díaz
needs-work Details | Review
Add timeout property to freeze, take 2 (4.60 KB, patch)
2011-05-24 14:37 UTC, Daniel Díaz
needs-work Details | Review

Description Daniel Díaz 2009-12-17 23:46:27 UTC
Created attachment 149949 [details] [review]
Add timeout property to freeze

The attached patch adds a property to the freeze element to end after a timeout o 'x' seconds occurs. This comes in handy when prototyping decoding of still images.
Comment 1 Tim-Philipp Müller 2009-12-18 09:01:56 UTC
Review of attachment 149949 [details] [review]:

::: 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@neteyes.hu>,"
+    "Renato Filho <renato.filho@indt.org.br>,"
+    "Daniel Diaz <yosoy@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.
Comment 2 Sebastian Dröge (slomo) 2011-05-23 14:27:14 UTC
Is there still interest in this patch? There's also the imagefreeze element in gst-plugins-good now, which does the same as freeze but better ;)
Comment 3 Daniel Díaz 2011-05-24 14:37:00 UTC
(In reply to comment #2)
> Is there still interest in this patch? There's also the imagefreeze element in
> gst-plugins-good now, which does the same as freeze but better ;)

Cool! Still, imagefreeze doesn't finish the stream by itself, which was the purpose of this patch.

This, again, is just for quick prototyping, as in acquiring an image from a video sensor, see it for some seconds, and be done with it. Can also be used with quick image decoding or cases like that.
Comment 4 Daniel Díaz 2011-05-24 14:37:29 UTC
Created attachment 188464 [details] [review]
Add timeout property to freeze, take 2

Address feedback (thanks!) from Tim, rebase to 0.10.22.
Comment 5 Sebastian Dröge (slomo) 2011-05-25 08:31:21 UTC
imagefreeze ends the stream by itself if you send a SEEK event to it with a stop position
Comment 6 Sebastian Dröge (slomo) 2011-05-25 08:33:04 UTC
You didn't address the g_timeout_add() comment though, this definitely has to be fixed before this patch can be accepted.
Comment 7 Tim-Philipp Müller 2011-05-25 09:29:04 UTC
Shouldn't the timeout be "stream time" based, ie. "stop after pushing out X seconds worth of frames", rather than "stop after X seconds have passed according to the clock on the wall"?
Comment 8 Akhil Laddha 2011-07-07 05:25:39 UTC
Daniel, could you please provide reworked patch including comment#6 and comment#7 ?
Comment 9 Daniel Díaz 2011-07-07 05:45:30 UTC
(In reply to comment #5)
> imagefreeze ends the stream by itself if you send a SEEK event to it with a
> stop position

Cool, but for gst-launch it still needs to terminate somehow, which means human interaction.

The original purpose of this patch, in the spirit of gst-launch's quick prototyping, was to enable a quick tool to visualize acquired images.

If imagefreeze works with gst-auto-launch (which can request state transitions on timed stages), I'll definitely use that instead!
Comment 10 Daniel Díaz 2011-07-07 05:49:10 UTC
(In reply to comment #8)
> Daniel, could you please provide reworked patch including comment#6 and
> comment#7 ?

Appy-polly-logies, but I don't think I'll have that anytime soon. It's not even on my to-do list anymore.

Please close this bug or let somebody else work on it if there's still any interest on it.