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 704774 - appsrc: add "current-level-bytes" property
appsrc: add "current-level-bytes" property
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.x
Other Linux
: Normal normal
: 1.1.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-07-24 01:09 UTC by Changbok
Modified: 2013-07-26 09:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add "queued-byte" action signal patch (3.44 KB, patch)
2013-07-24 01:09 UTC, Changbok
needs-work Details | Review
change "queued-byte" action signal to property (9.70 KB, patch)
2013-07-25 11:31 UTC, Changbok
needs-work Details | Review
change property name to "current_level_byte" (3.82 KB, patch)
2013-07-26 01:37 UTC, Changbok
none Details | Review
change to static function (3.55 KB, patch)
2013-07-26 06:13 UTC, Changbok
committed Details | Review
appsrc : add "current-level-bytes" property (3.56 KB, patch)
2013-07-26 08:45 UTC, Changbok
rejected Details | Review

Description Changbok 2013-07-24 01:09:12 UTC
Created attachment 249982 [details] [review]
add "queued-byte" action signal patch

when we configure custom bin via appsrc element, we can't get appsrc buffer's queued bytes(remained size). Although appsrc has "need-data"and"enough-data" signal about appbuffer control, we need appsrc buffer's remained size to check total appsrc element pipeline's buffer percentage.
 this action signal("queued-byte") help to get appsrc buffer's remained size.
below code is example.

guint64 queuedSize = 0;

g_signal_emit_by_name(pAppSrcElement, "queued-byte", &queuedSize);
Comment 1 Sebastian Dröge (slomo) 2013-07-24 07:17:01 UTC
Comment on attachment 249982 [details] [review]
add "queued-byte" action signal patch

Just make this a normal GObject property, no need for it to be an action signal :)

Why exactly do you need it? To implement non-blocking pushing to appsrc but still have a queue inside appsrc? For that it would alternatively be possible to just add a property to appsrc to return a custom flow return as result of push_buffer() if the queue would run full.
Comment 2 Changbok 2013-07-25 11:31:50 UTC
Created attachment 250100 [details] [review]
change "queued-byte" action signal to property

I agree your opinion. signal is not appropriate when user needs some information query. so I changed signal method to property.

I tried to make app for streaming play. also I'd like to prebuffer in app. because if app prebuffered enough data, play will be smoothly.
although appsrc already had "enough-data" signal, I should know appsrc buffer's current remained size, because app has threshold about pipeline(custom bin) total buffer.
Comment 3 Sebastian Dröge (slomo) 2013-07-25 12:36:45 UTC
Review of attachment 250100 [details] [review]:

Don't call gst-indent on headers, and don't change the header at all please

::: gst-libs/gst/app/gstappsrc.c
@@ +385,2 @@
   /**
+   * GstAppSrc::queued-byte:

I'd call it current-bytes instead, similar to what is used in queue for example

@@ -456,3 @@
-    *
-    */
-  gst_app_src_signals[SIGNAL_QUEUED_BYTE] =

Please provide a patch directly against git master, excluding your previous changes

@@ +1563,1 @@
 gst_app_src_get_queued_bytes (GstAppSrc * appsrc)

This should stay static
Comment 4 Changbok 2013-07-26 01:37:17 UTC
Created attachment 250167 [details] [review]
change property name to "current_level_byte"

I changed property name to "current_level_byte"(same with queue "current_level_byte").
Also not using gst-indent, compare with master branch.

thank you
Comment 5 Justin Kim 2013-07-26 02:07:36 UTC
Review of attachment 250167 [details] [review]:

::: gst-libs/gst/app/gstappsrc.c
@@ +1562,3 @@
+ * This is used for the action signal. */
+guint64
+ *

I think, this function should be static because you've already provided an access point by g_object_get_property.
Comment 6 Changbok 2013-07-26 06:13:56 UTC
Created attachment 250177 [details] [review]
change to static function 

I changed "gst_app_src_get_queued_bytes" function to static. so modified some description.
Comment 7 Changbok 2013-07-26 08:45:37 UTC
Created attachment 250186 [details] [review]
appsrc : add "current-level-bytes" property

I modified "current-level-byte" property name to "current-level-bytes"
Comment 8 Sebastian Dröge (slomo) 2013-07-26 09:03:44 UTC
commit e2597e1e42d91c2bba2c09b1b2900eb7a38d1b2e
Author: Sebastian Dröge <slomo@circular-chaos.org>
Date:   Fri Jul 26 11:02:32 2013 +0200

    appsrc: Also provide function API for current-level-bytes and integrate into the docs

commit bdbfa45296d41b984a60842510cd6a27ed7bad4a
Author: Changbok Chea <changbok.chea@lge.com>
Date:   Fri Jul 26 15:00:44 2013 +0900

    appsrc: Add "current-level-bytes" property
    
    https://bugzilla.gnome.org/show_bug.cgi?id=704774
Comment 9 Sebastian Dröge (slomo) 2013-07-26 09:04:10 UTC
Comment on attachment 250186 [details] [review]
appsrc : add "current-level-bytes" property

Fixed that already, but thanks :)