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 781393 - tsdemux: fix to keep seqnum of segment
tsdemux: fix to keep seqnum of segment
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-04-17 07:34 UTC by Wonchul Lee
Modified: 2018-11-03 14:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tsdemux: fix to keep seqnum of segment (4.47 KB, patch)
2017-04-17 07:35 UTC, Wonchul Lee
none Details | Review
tsdemux: fix to keep seqnum of segment #2 (5.23 KB, patch)
2017-04-20 08:48 UTC, Wonchul Lee
none Details | Review
tsdemux: fix to keep seqnum of segment #3 (5.27 KB, patch)
2017-04-20 11:02 UTC, Wonchul Lee
needs-work Details | Review

Description Wonchul Lee 2017-04-17 07:34:35 UTC
I tried to seek on hls stream, here is the url : https://mnmedias.api.telequebec.tv/m3u8/29880.m3u8

Hlsdemux handled the seek event and generated relevant flush-start/stop and segment event, but the tsdemux didn't keep the seqnum of segment which receiving event from hlsdemux in this case. So I fixed to keep seqnum of segment in tsdemux for this.
Comment 1 Wonchul Lee 2017-04-17 07:35:46 UTC
Created attachment 349932 [details] [review]
tsdemux: fix to keep seqnum of segment
Comment 2 Sebastian Dröge (slomo) 2017-04-19 09:33:09 UTC
Review of attachment 349932 [details] [review]:

::: gst/mpegtsdemux/mpegtsbase.c
@@ +203,2 @@
   gst_segment_init (&base->segment, GST_FORMAT_UNDEFINED);
+  base->segment_seqnum = (guint32) 0;

Why the cast?

@@ +216,3 @@
   GST_DEBUG_OBJECT (base, "Streams aware : %d", base->streams_aware);
 
+  base->segment_seqnum = 0;

Why are you setting it to 0 twice?

::: gst/mpegtsdemux/tsdemux.c
@@ +1721,3 @@
+        if (base->segment_seqnum) {
+          gst_event_set_seqnum (event, base->segment_seqnum);
+          base->segment_seqnum = 0;

Why do you reset it to 0? And there are other places in tsdemux and mpegtsbase where EOS events are send

@@ +2379,3 @@
   if (!demux->segment_event) {
     demux->segment_event = gst_event_new_segment (&demux->segment);
+    GST_EVENT_SEQNUM (demux->segment_event) = base->segment_seqnum;

gst_event_set_seqnum()
Comment 3 Wonchul Lee 2017-04-20 08:48:19 UTC
Created attachment 350105 [details] [review]
tsdemux: fix to keep seqnum of segment #2

I made some mistakes, fixed it and added to set seqnum when generating EOS event in tsdemux also, thanks!
Comment 4 Wonchul Lee 2017-04-20 11:02:14 UTC
Created attachment 350113 [details] [review]
tsdemux: fix to keep seqnum of segment #3

oops, I pushed the wrong patch, updated it
Comment 5 Edward Hervey 2018-05-12 07:09:24 UTC
Review of attachment 350113 [details] [review]:

Definitely good and needed.

Only nitpick. Could you use GST_SEQNUM_INVALID for initialization and checking whether it is valid ? This makes the code clearer.

And could you rebase it against current master ? Thanks !
Comment 6 GStreamer system administrator 2018-11-03 14:07:21 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/546.