[Bug 768510] appsrc: handle pushing custom segments

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=768510

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

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

--- Comment #7 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 331151:
 --> (https://bugzilla.gnome.org/review?bug=768510&attachment=331151)

Thanks, this looks like a good start :) We should double-check if the seamless
segment API of basesrc does the right thing though, it was implemented for a
completely different use-case. That's why it has the "seamless" in the name.
This here is not really for anything seamless anymore.


I'm not sure there should be a new property for this. It should be safe to just
always do that if the sample contains a valid segment

::: gst-libs/gst/app/gstappsrc.c
@@ +167,3 @@
 #define DEFAULT_PROP_CURRENT_LEVEL_BYTES   0
 #define DEFAULT_PROP_DURATION      GST_CLOCK_TIME_NONE
+#define DEFAULT_PROP_DRIVEN_BY_APPLICTAION FALSE

Typo, APPLICATION

@@ +185,3 @@
   PROP_CURRENT_LEVEL_BYTES,
   PROP_DURATION,
+  PROP_DRIVEN_BY_APPLICTAION,

Here too

@@ +427,3 @@
+   * GstAppSrc::driven-by-application:
+   *
+   * Allows applictaion to push custom segment with calling the "push-sample"

And here

@@ +1202,2 @@
       if (!GST_IS_BUFFER (obj)) {
+        if (!GST_IS_CAPS (obj)) {

Store the GstSample in the queue, and then use
gst_sample_get_buffer/caps/segment() here instead

::: tests/check/elements/appsrc.c
@@ +512,3 @@
+  GList *expected = NULL;
+
+  segment = gst_segment_new ();

Allocate on the stack :)

@@ +514,3 @@
+  segment = gst_segment_new ();
+
+  for (i = 0; i < sizeof (modes) / sizeof (modes[0]); i++) {

G_N_ELEMENTS

@@ +521,3 @@
+    sink = gst_element_factory_make ("fakesink", NULL);
+    gst_bin_add_many (GST_BIN (pipe), src, sink, NULL);
+    gst_element_link (src, sink);

Some error checking here

@@ +534,3 @@
+    gst_app_src_set_callbacks (GST_APP_SRC (src), &cb, NULL, NULL);
+
+    gst_element_set_state (pipe, GST_STATE_PLAYING);

Check if this returns an error

@@ +576,3 @@
+
+    /* Give some time to the appsrc loop to push the buffers */
+    g_usleep (G_USEC_PER_SEC * 1);

Why?

@@ +580,3 @@
+    if (modes[i] == GST_APP_STREAM_TYPE_SEEKABLE) {
+      /* Client request seek to 7 sec position (which belongs to 2nd period)
+       * Applictaion must provides corresponding buffer (of 2nd period) with

Typo, Application

@@ +626,3 @@
+    msg_types = GST_MESSAGE_EOS;
+
+    msg = gst_bus_timed_pop_filtered (GST_ELEMENT_BUS (pipe), -1, msg_types);

Should probably check for errors

@@ +643,3 @@
+
+
+GST_START_TEST (test_appsrc_custom_segment_twice)

Same comments as above

@@ +804,3 @@
+   * - Application has been notified that it can control pipeline timeline
+   *   (by setting 'driven-by-application' property)
+   * - Both appsrc segment and the custom segment have TIME format

BYTES format and BYTES segment and other compatible pairs should also work
though

@@ +892,3 @@
+      /* 1st sample includes buffer and segment */
+      fail_unless (gst_app_src_push_sample (GST_APP_SRC (src), sample)
+          == GST_FLOW_OK);

Shouldn't this give a GST_FLOW_ERROR then?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.