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 645006 - [mpegtsmux] in m2ts-mode, PAT is written only once
[mpegtsmux] in m2ts-mode, PAT is written only once
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal major
: 0.10.22
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-03-17 11:01 UTC by Andreas Frisch
Modified: 2011-05-24 15:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix (PAT) packet loss (1.15 KB, patch)
2011-03-17 11:02 UTC, Andreas Frisch
none Details | Review
mpegtsmux: Correct periodical injection of PAT Fixes bug #645006 (1.54 KB, patch)
2011-03-21 16:05 UTC, Andreas Frisch
none Details | Review
Correct periodical injection of PAT (1.67 KB, patch)
2011-03-21 16:25 UTC, Andreas Frisch
none Details | Review

Description Andreas Frisch 2011-03-17 11:01:59 UTC
this resulted in hardware demuxer sometimes not being able to start filtering because it didn't see the first and only appearing PAT in the m2ts stream
so i debugged and found two reasons for the PAT only being written once
the first is in the underlying tsmuxer.c where the PAT is only generated if a flag mux->pat_changed is set but this flag is becoming TRUE again after initialization.
the first problem is that in the new_packet_cb, every very first packet with a new_pcr gets gets lost because it's not being put into the adapter. since PAT is usually only one TS packet long and has a new_pcr, every PAT except for the first is affected (since the first_pcr packet is directly pushed without adapter).
i hope i did this patch right and i'd appreciate it if you could review it.
Comment 1 Andreas Frisch 2011-03-17 11:02:30 UTC
Created attachment 183616 [details] [review]
fix (PAT) packet loss
Comment 2 Andreas Frisch 2011-03-21 16:05:26 UTC
Created attachment 183956 [details] [review]
mpegtsmux: Correct periodical injection of PAT Fixes bug #645006
Comment 3 Andreas Frisch 2011-03-21 16:25:56 UTC
Created attachment 183961 [details] [review]
Correct periodical injection of PAT
Comment 4 Jan Schmidt 2011-03-26 07:12:35 UTC
This seems to be completely fixed with this commit, despite the commit msg only saying partially fixed:

commit 9a26173a57b05ecb497fe1fca0168b5cbd4c0167
Author: Jan Schmidt <thaytan@noraisin.net>
Date:   Sat Mar 26 15:58:21 2011 +1100

    Rewrite M2TS packet output
    
    Make sure we only write the bottom 30 bits of the PCR to the m2ts header.
    Don't use floating point computation for it, and remove weird bit fiddling
    that messes up the PCR in a way I can't find any
    justification/documentation for.
    
    Don't accidentally lose PCR packets from the output.
    
    Fix the description for the m2ts-mode property so it's clear it's a flag,
    and which setting does what.
    
    Fixes: #611061 #644429
    Partially fixes: #645006

There's no need to disable the pat_changed flag in the tsmux, it's set when a new program is added to the set, to indicate the PAT needs re-generating, otherwise the old PAT is just output.
 
Please re-open if you see any other trouble.
Comment 5 Andreas Frisch 2011-05-24 15:43:23 UTC
thaytan, i have to reopen this issue since the part of the problem, the continuous pat injection, persists.

the mux->pat_changed flag in tsmux.c still doesn't ever become true after the very first time

diff --git a/gst/mpegtsmux/tsmux/tsmux.c b/gst/mpegtsmux/tsmux/tsmux.c
index 1a50e72..66f648a 100644
--- a/gst/mpegtsmux/tsmux/tsmux.c
+++ b/gst/mpegtsmux/tsmux/tsmux.c
@@ -887,7 +887,7 @@ tsmux_write_pat (TsMux * mux)
   GList *cur;
   TsMuxSection *pat = &mux->pat;
 
-  if (mux->pat_changed) {
+  /*if (mux->pat_changed) */  {
     /* program_association_section ()
      * table_id                                   8   uimsbf
      * section_syntax_indicator                   1   bslbf
Comment 6 Andreas Frisch 2011-05-24 15:45:05 UTC
sorry, it seems it works with re-injecting though!