[Bug 736655] basesink: preroll issue for some clips which audio is shorter than video

GStreamer (GNOME Bugzilla) bugzilla at gnome.org
Fri Feb 13 00:50:42 PST 2015


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

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

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

Looks good, just some minor things that should be fast to update :) Then this
should be ready to be merged

::: gst/playback/gststreamsynchronizer.c
@@ +207,3 @@
+/* must be called with the STREAM_SYNCHRONIZER_LOCK */
+static gboolean
+gst_stream_synchronizer_wait (GstStreamSynchronizer * self, GstPad * pad)

Can this function also be used for the g_cond_wait() in the SEGMENT event
handler? I think it should be used there too for consistency (and modified to
work in both cases).

@@ +445,3 @@
       stream = gst_pad_get_element_private (pad);
+      stream->flushing = TRUE;

See the "if (stream)" some lines below :) You need to check for NULL

@@ +550,3 @@
+      /* send eos if haven't seed data as sink can't handle if haven't
negotiate */
+      if (!seen_data || self->eos) {

The only case when we get here and this is not true, is when we're flushing?
Document that and maybe even add an assertion :)

@@ +904,3 @@
+        if (stream->is_eos && !stream->eos_sent) {
+          self->send_gap_event = TRUE;
+          g_cond_broadcast (&stream->stream_finish_cond);

Please document in a comment here why a GAP event should be sent, also why not
another one is required when going from PAUSED to PLAYING

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

More information about the gstreamer-bugs mailing list