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 728356 - jpegparse: Does not passthrough timestamps depending on downstream elements
jpegparse: Does not passthrough timestamps depending on downstream elements
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal major
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-16 16:45 UTC by Nicola
Modified: 2018-11-03 13:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bytereader: add gst_byte_reader_masked_scan_uint32_peek (7.84 KB, patch)
2014-04-24 04:18 UTC, Thiago Sousa Santos
reviewed Details | Review
jpegparse: port to baseparse (24.17 KB, patch)
2014-04-24 04:18 UTC, Thiago Sousa Santos
none Details | Review
bytereader: add gst_byte_reader_masked_scan_uint32_peek (5.06 KB, patch)
2014-07-24 15:48 UTC, Thiago Sousa Santos
none Details | Review

Description Nicola 2014-04-16 16:45:43 UTC
please test this pipeline:

gst-launch-0.10 -v souphttpsrc location=http://213.251.201.196/anony/mjpg.cgi do-timestamp=true ! jpegparse ! videorate ! image/jpeg,framerate=3/1 ! fakesink silent=false

you'll see:

/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = "chain   ******* (fakesink0:sink) (8060 bytes, timestamp: 0:00:00.333333333, duration: 0:00:00.333333333, offset: 1, offset_end: 2, flags: 1 ro ) 0x7f33d0025a30"
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = "chain   ******* (fakesink0:sink) (8042 bytes, timestamp: 0:00:00.666666666, duration: 0:00:00.333333334, offset: 2, offset_end: 3, flags: 1 ro ) 0x7f33d0025ad0"
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = "chain   ******* (fakesink0:sink) (8072 bytes, timestamp: 0:00:01.000000000, duration: 0:00:00.333333333, offset: 3, offset_end: 4, flags: 1 ro ) 0x7f33d0008f30

now the same in 1.0 (today git):

gst-launch-1.0 -vm souphttpsrc location=http://213.251.201.196/anony/mjpg.cgi do-timestamp=true ! jpegparse ! videorate ! image/jpeg,framerate=3/1 ! fakesink silent=false

no buffer arrive to fakesink the logs show these messages:

0:00:04.502834927  4274      0x1678850 WARN               videorate gstvideorate.c:1109:gst_video_rate_transform_ip:<videorate0> Got buffer with GST_CLOCK_TIME_NONE timestamp, discarding it
0:00:04.510393982  4274      0x1678850 WARN               videorate gstvideorate.c:1109:gst_video_rate_transform_ip:<videorate0> Got buffer with GST_CLOCK_TIME_NONE timestamp, discarding it
Comment 1 Nicola 2014-04-16 16:49:31 UTC
sorry, works in 1.0 master setting live=true on souphttpsrc, error out in 1.2
Comment 2 Sebastian Dröge (slomo) 2014-04-17 07:53:29 UTC
You don't even need the capsfilter :)

Not sure why this happens, jpegparse outputs PTS on all buffers if running without videorate. If having a videorate after it, it will put NONE timestamps on buffers.

Probably a regression in baseparse.
Comment 3 Thiago Sousa Santos 2014-04-21 05:52:18 UTC
jpegparse isn't based on baseparse.

The issue here is that videorate will drop all buffers with PTS=none and as it drops all buffers the pipeline never prerolls and there is no clock.

Because of this the source will never timestamp elements and videorate will continue dropping buffers.

The solution here is to make jpegparse timestamp the buffers itself by using the selected framerate, but only when upstream is using a bytes segment. If it uses a time segment it should respect upstream PTS.
Comment 4 Thiago Sousa Santos 2014-04-24 04:18:19 UTC
Created attachment 275017 [details] [review]
bytereader: add gst_byte_reader_masked_scan_uint32_peek

Adds gst_byte_reader_masked_scan_uint32_peek just like
GstAdapter has a _peek and non _peek version

API: gst_byte_reader_masked_scan_uint32_peek
Comment 5 Thiago Sousa Santos 2014-04-24 04:18:44 UTC
Created attachment 275018 [details] [review]
jpegparse: port to baseparse
Comment 6 Thiago Sousa Santos 2014-04-24 04:20:12 UTC
Nicola, what is exactly your use case here?

Those patches I attached port jpegparse to baseparse, but your issue is still present after porting it. Do you want the source to timestamp the buffers with the running time as they are received? Do you want to just set a framerate and have PTS derived from that? Better understanding your scenario will help us find a proper fix.
Comment 7 Nicola 2014-04-24 06:56:23 UTC
Thiago, thanks for your work! 

I want the source to timestamp the received buffers with the running time and then I want to use videorate to drop/duplicate buffers based on a capsfilter, please see the output produced by 0.10:

- first buffer ts 0:00:00.333333333
- second buffer ts 0:00:00.666666666
- third buffer ts 0:00:01.000000000

in my use case I have a source that output about 25 fps and I want to retrasmit it at lower fps without reencoding, in 1.0 this seems to work settings is-live=true on souphttpsrc, in 0.10 it works with and without is-live=true on the source
Comment 8 Thiago Sousa Santos 2014-04-28 17:28:43 UTC
My take on this is to let videorate pass buffers with GST_CLOCK_TIME_NONE at the beginning of the stream or resort to the DTS that IIRC basesrc sets to 0.

Basesrc could timestamp buffers with 0 before it gets a clock, but this also seems wrong as there would be multiple buffers with timestamp=0.
Comment 9 Thiago Sousa Santos 2014-05-30 16:30:33 UTC
Any opinions?
Comment 10 Tim-Philipp Müller 2014-05-30 16:55:23 UTC
Comment on attachment 275017 [details] [review]
bytereader: add gst_byte_reader_masked_scan_uint32_peek

> /* Special optimized scan for mask 0xffffff00 and pattern 0x00000100 */
> static inline gint
>-_scan_for_start_code (const guint8 * data, guint offset, guint size)
>+_scan_for_start_code (const guint8 * data, guint offset, guint size,
>+    guint32 * value)

Did you measure if the addition of this has a negative performance impact on the non-peek function? (i.e. if the compiler actually inlines it and optimises it away or not)


>+    if (value)
>+      *value = *((guint32 *) (data + i + offset));

This looks problematic for two reasons: are we guaranteed aligned access here? what about endianness? Is it always native? That seems unlikely, but I haven't looked at the code.

> /**
>- * gst_byte_reader_masked_scan_uint32:
>+ * gst_byte_reader_masked_scan_uint32_peek:
>  * @reader: a #GstByteReader
>  * @mask: mask to apply to data before matching against @pattern
>  * @pattern: pattern to match (after mask is applied)
>  * @offset: offset from which to start scanning, relative to the current
>  *     position
>  * @size: number of bytes to scan from offset
>+ * @value: pointer to uint32 to return matching data
>  *
>  * Scan for pattern @pattern with applied mask @mask in the byte reader data,
>  * starting from offset @offset relative to the current position.
>@@ -818,29 +823,11 @@ _scan_for_start_code (const guint8 * data, guint offset, guint size)
>  *
>  * Returns: offset of the first match, or -1 if no match was found.
>  *
>- * Example:
>- * <programlisting>
>- * // Assume the reader contains 0x00 0x01 0x02 ... 0xfe 0xff
>- *
>- * gst_byte_reader_masked_scan_uint32 (reader, 0xffffffff, 0x00010203, 0, 256);
>- * // -> returns 0
>- * gst_byte_reader_masked_scan_uint32 (reader, 0xffffffff, 0x00010203, 1, 255);
>- * // -> returns -1
>- * gst_byte_reader_masked_scan_uint32 (reader, 0xffffffff, 0x01020304, 1, 255);
>- * // -> returns 1
>- * gst_byte_reader_masked_scan_uint32 (reader, 0xffff, 0x0001, 0, 256);
>- * // -> returns -1
>- * gst_byte_reader_masked_scan_uint32 (reader, 0xffff, 0x0203, 0, 256);
>- * // -> returns 0
>- * gst_byte_reader_masked_scan_uint32 (reader, 0xffff0000, 0x02030000, 0, 256);
>- * // -> returns 2
>- * gst_byte_reader_masked_scan_uint32 (reader, 0xffff0000, 0x02030000, 0, 4);
>- * // -> returns -1
>- * </programlisting>
>+ * Since: 1.4
>  */
> guint
>-gst_byte_reader_masked_scan_uint32 (const GstByteReader * reader, guint32 mask,
>-    guint32 pattern, guint offset, guint size)
>+gst_byte_reader_masked_scan_uint32_peek (const GstByteReader * reader,
>+    guint32 mask, guint32 pattern, guint offset, guint size, guint32 * value)

Maybe you could move the new _peek() function behind the other one, so we don't have all this diff churn? :)
Comment 11 Tim-Philipp Müller 2014-05-30 16:58:41 UTC
As for the port to baseparse, I think you should just push that if you have done some testing on it, since that's definitely the direction we want to go in, and it's not autoplugged anyway for now (and I would want to keep it that way for now).
Comment 12 Nicolas Dufresne (ndufresne) 2014-05-30 18:38:50 UTC
Review of attachment 275017 [details] [review]:

::: libs/gst/base/gstbytereader.c
@@ +775,3 @@
 static inline gint
+_scan_for_start_code (const guint8 * data, guint offset, guint size,
+    guint32 * value)

I think you do the peek outside of this method. And the _peek() version would call the none peek() version instead.
Comment 13 Thiago Sousa Santos 2014-07-24 15:48:37 UTC
Created attachment 281612 [details] [review]
bytereader: add gst_byte_reader_masked_scan_uint32_peek

New version that applies against master cleanly
Comment 14 Thiago Sousa Santos 2014-07-24 20:14:41 UTC
commit f8323b17fb2640e90b10dbe98ac7516bbf381908
Author: Thiago Santos <ts.santos@osg.sisa.samsung.com>
Date:   Thu Jul 24 14:39:11 2014 -0300

    bytereader: add gst_byte_reader_masked_scan_uint32_peek
    
    Adds gst_byte_reader_masked_scan_uint32_peek just like
    GstAdapter has a _peek and non _peek version
    
    Upgraded tests to check that the returned value is correct in the
    _peek version
    
    API: gst_byte_reader_masked_scan_uint32_peek
    
    https://bugzilla.gnome.org/show_bug.cgi?id=728356

commit 2bfd106ef603f2888b634a2be0cd62a30c6a87d3
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Mon Apr 21 23:05:48 2014 -0300

    jpegparse: port to baseparse
    
    https://bugzilla.gnome.org/show_bug.cgi?id=728356


Patches to port baseparse pushed. Now we need to decide on a solution to the original issue.
Comment 15 GStreamer system administrator 2018-11-03 13:22:31 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/140.