GNOME Bugzilla – Bug 728356
jpegparse: Does not passthrough timestamps depending on downstream elements
Last modified: 2018-11-03 13:22:31 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
sorry, works in 1.0 master setting live=true on souphttpsrc, error out in 1.2
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.
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.
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
Created attachment 275018 [details] [review] jpegparse: port to baseparse
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.
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
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.
Any opinions?
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? :)
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).
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.
Created attachment 281612 [details] [review] bytereader: add gst_byte_reader_masked_scan_uint32_peek New version that applies against master cleanly
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.
-- 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.