[Bug 762715] mpegtsmux: alignment property and timestamps for Live UDP streaming

[GStreamer (GNOME Bugzilla)]

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

Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> changed:

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

--- Comment #7 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> ---
Review of attachment 322640:
 --> (https://bugzilla.gnome.org/review?bug=762715&attachment=322640)

Looks like the way forward, though I'd like to see a unit test to go with it.
Also, are you sure the timestamp code is right? gst_adapter_take_buffer() will
flush the buffers, possibly removing the timestamp you where looking for. I
would have expected the timestamp to be read before flushing the data form the
adapter. As this shrink the adapter, I would expected a fixed offset from start
(align or align - 1, not sure if prev is exclusive).

::: /a/gst/mpegtsmux/mpegtsmux.c
@@ +1524,3 @@
+    /* first sync on DTS, else use PTS */
+    ts = gst_adapter_prev_dts_at_offset (mux->out_adapter, offset, NULL);
+    if (!GST_CLOCK_TIME_IS_VALID (ts)) {

This code is correct, but as I got confused reading it, maybe it's better to
avoid the not (!) here and flip and cases ?

@@ +1528,3 @@
+      GST_BUFFER_PTS (buf) = ts;  
+    }
+    else {

Wrong style, should be "} else {" for GStreamer code. Do you have indent
installed properly ? We normally have commit hooks that catches these.

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