GNOME Bugzilla – Bug 639063
mpegtsparse: fix (re)sync with invalid data at beginning
Last modified: 2011-01-10 12:18:23 UTC
Created attachment 177871 [details] [review] Bug fix proposal mpegtsparse is not handling seeks correctly. In my case when I am playing MPEG-TS from file and then trying to seek, the mpegtsparse is holding data (not pushing it further). After seek the information about transport stream packet size is lost. Then in function "mpegts_try_discover_packet_size" the packet size is obtained again (when new data in arriving). If the packets at the beginning of "gst_adapter" are invalid, the "mpegts_try_discover_packet_size" would not be able to determine the packet size and the packets at the beginning of the gst_adapter will not be discarded. Therefore all incoming data will be queued in gst_adapter indefinitely. I have attached a proposal fix for this bug
Comment on attachment 177871 [details] [review] Bug fix proposal >diff --git a/gst/mpegdemux/mpegtspacketizer.c b/gst/mpegdemux/mpegtspacketizer.c >index 70cf3d7..0792b6d 100644 >--- a/gst/mpegdemux/mpegtspacketizer.c >+++ b/gst/mpegdemux/mpegtspacketizer.c >@@ -2108,8 +2118,13 @@ mpegts_try_discover_packet_size (MpegTSPacketizer * packetizer) > GST_DEBUG ("have packetsize detected: %d of %u bytes", > packetizer->know_packet_size, packetizer->packet_size); > /* flush to sync byte */ >- if (pos > 0) >+ if (pos > 0) { >+ /* flush to sync byte */ > gst_adapter_flush (packetizer->adapter, pos); >+ } else { >+ /* drop some invalid data and move to the next possible packets */ >+ gst_adapter_flush (packetizer->adapter, MPEGTS_MAX_PACKETSIZE); >+ } > g_free (dest); > } > Are you sure this is correct? .... } else if (pos < 0) { gst_adapter_flush (packetizer->adapter, MPEGTS_MAX_PACKETSIZE); } ... would make more sense to me.
I think it should have actually be: /* flush to sync byte */ if (pos > 0) { /* flush to sync byte */ gst_adapter_flush (packetizer->adapter, pos); } else if (!packetizer->know_packet_size) { /* drop some invalid data and move to the next possible packets */ gst_adapter_flush (packetizer->adapter, MPEGTS_MAX_PACKETSIZE); }
Created attachment 177908 [details] [review] Bug fix proposal 2 /* flush to sync byte */ if (pos > 0) { /* flush to sync byte */ gst_adapter_flush (packetizer->adapter, pos); } else if (!packetizer->know_packet_size) { /* drop some invalid data and move to the next possible packets */ gst_adapter_flush (packetizer->adapter, MPEGTS_MAX_PACKETSIZE); }
Same thing really... both work for me. Now could you please tell us your full name so we can attribute the patch properly?
My name is: Karol Sobczak. I think it should also be in my profile now.
Fixed, thanks! commit 0b4dfa685d313f698c3b6b05d5f6894c20bfbc38 Author: Karol Sobczak <napewnotrafi@gmail.com> Date: Mon Jan 10 11:18:52 2011 +0000 mpegtsdemux: fix re-syncing on invalid data after seek Or possibly even at startup. If we couldn't find a sync within the first few bytes, we'd just push more data into the adapter but never discard any of the invalid data at the beginning, so would never be able to re-sync. https://bugzilla.gnome.org/show_bug.cgi?id=639063