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 653481 - tsdemux: Fail to sync to stream when input buffers lacks OFFSET
tsdemux: Fail to sync to stream when input buffers lacks OFFSET
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
0.10.22
Other other
: Normal normal
: 0.10.23
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-06-27 12:53 UTC by Jonas Larsson
Modified: 2011-07-14 22:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] tsdemux: Set first_pat_offset to 0 if the PAT buffer doesn't have a valid offset (1.10 KB, patch)
2011-06-30 23:03 UTC, Youness Alaoui
rejected Details | Review

Description Jonas Larsson 2011-06-27 12:53:40 UTC
gst-launch -v souphttpsrc location=http://devimages.apple.com/iphone/samples/bipbop/bipbopall.m3u8 ! hlsdemux ! tsdemux ! fakesink

mpegts_base_apply_pmt: WARN Got pmt without pat first. Returning

first_pat_offset is -1 because input buffer has offset -1. PAT is already parsed though. If this check is removed the stream plays fine.
Comment 1 Youness Alaoui 2011-06-30 23:03:49 UTC
Created attachment 191063 [details] [review]
[PATCH] tsdemux: Set first_pat_offset to 0 if the PAT buffer doesn't have a valid offset

Here's a workaround fix to this bug.
The bug is actually from hlsdemux, it doesn't set the GST_BUFFER_OFFSET on the buffer while tsdemux expects it to be defined.
The attached patch will assume the offset is 0 if it's not defined. The proper fix would be to set it in hlsdemux, but for synchronization reasons, hlsdemux needs a lot more changes than simply setting the buffer offset.
Comment 2 Edward Hervey 2011-07-01 06:57:49 UTC
While I agree this fixes the issue, there's a couple of reason I'm not keen on pushing this one.
* As you mentionned the issue might be cleaning up hlsdemux...
* .. but also that tsdemux should detect that the source isn't seekable and therefore not have to care about the offset of the first PAT, but rather care whether it saw one or not

I'd accept a patch that modified mpegtsbase for better detecting first PAT presence.
Comment 3 Tim-Philipp Müller 2011-07-08 11:22:21 UTC
> * .. but also that tsdemux should detect that the source isn't seekable (...)

There's a plan to implement seeking at some point, fwiw.
Comment 4 Edward Hervey 2011-07-14 20:13:02 UTC
push-based seeking might require fixups later on (depending on how it's handled for hls use-cases).


commit a82483e36772467de9d00dcca52214bcf548df6a
Author: Edward Hervey <bilboed@bilboed.com>
Date:   Thu Jul 14 22:08:56 2011 +0200

    mpegtsbase: Split up whether we saw a PAT and its offset
    
    Fixes the issue with streams that don't set an offset on their buffers,
    like those coming from hlsdemux.
    
    Fixes #653481
Comment 5 Alessandro Decina 2011-07-14 22:07:54 UTC
... and

commit d439f2d38d590a47e60e5b4d7fd7a2f2307beac6
Author: Alessandro Decina <alessandro.d@gmail.com>
Date:   Fri Jul 15 00:03:10 2011 +0200

    mpegtsbase: actually set seen_pat=TRUE when we see a PAT

diff --git a/gst/mpegtsdemux/mpegtsbase.c b/gst/mpegtsdemux/mpegtsbase.c
index a2e7cba..463090a 100644
--- a/gst/mpegtsdemux/mpegtsbase.c
+++ b/gst/mpegtsdemux/mpegtsbase.c
@@ -834,7 +834,7 @@ mpegts_base_handle_psi (MpegTSBase * base, MpegTSPacketizerSection * section)
       if (G_LIKELY (structure)) {
         mpegts_base_apply_pat (base, structure);
         if (base->seen_pat == FALSE) {
-
+          base->seen_pat = TRUE;
           base->first_pat_offset = GST_BUFFER_OFFSET (section->buffer);
           GST_DEBUG ("First PAT offset: %" G_GUINT64_FORMAT,
               base->first_pat_offset);