Opened 18 years ago
Closed 17 years ago
#2147 closed defect (fixed)
pes assembler
Reported by: | Owned by: | Stuart Auchterlonie | |
---|---|---|---|
Priority: | minor | Milestone: | 0.21 |
Component: | dvb | Version: | head |
Severity: | medium | Keywords: | |
Cc: | Ticket locked: | no |
Description
Attached patch addresses following issues:
Ensure that _psiOffset + 3 + 1 is already read before parsing a pes packet header (mainly calling Length()). This fixes several issues e.g. EIT events with a starttime in 2038, segfault int PAT parsing or "program number not found in PAT" errors.
Fixes pesdata calculation in InitPESPacket for PESPacket::View().
Removes a useless statement from AssemblePSIP().
Attachments (7)
Change History (35)
Changed 18 years ago by
Attachment: | pes_assembler_fixes.diff added |
---|
comment:1 Changed 18 years ago by
Owner: | changed from Stuart Auchterlonie to danielk |
---|
comment:3 Changed 18 years ago by
Milestone: | → 0.20 |
---|
Replying to anonymous:
and I see sometimes a bunch of "AddTSPacket: Out of sync!!! Need to wait for next payloadStart PID: 0x12, continuity counter: 6 (expected 5)."
The continuity counter is always expected+1.
This would be a symptom of a occasional signal glitch and this is the point of the CC. The CC should cycle through 0-15 and if a TS packet is corrupted it'll be dropped leading to a discontinuity in the CC. This gives you the information needed to drop that PES packet due to bad signal or intermittent error.
comment:4 Changed 18 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [10734]) Fixes #2147. Various PES packet assembly fixes.
This prevents printing spurious warnings with the pespacket.cpp changes, fixes an offset problem in pespacket.h affecting DVB standard PSIP, checks the PES packet size before tring to verify it in AssemblePSIP, and also checks the PES packet size when it is supposed to be complete so that one scrambled packet can't disrupt the decoding of the next packet on the same PID.
comment:5 Changed 18 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Type: | patch → defect |
Changed 18 years ago by
Attachment: | pespacket_assembler_debug.diff added |
---|
comment:6 Changed 18 years ago by
valgrind will only detect incomplete packets as uninitialized data if we disable the pes allocator.
So run the backend under valgrind only with pespacket_assembler_debug.diff applied.
comment:7 Changed 18 years ago by
Owner: | changed from danielk to Stuart Auchterlonie |
---|---|
Status: | reopened → new |
Stuart I hate to offload this on you, but if someone has a safe fix before 0.20...
BTW It's also possible that the problem is not with the packet assembler. We may not be checking the CRC on every SI table because we may be checking the PES CRC flag which isn't valid for some of the tables... Or, the users reporting problems may be using one of the cards blacklisted for CRC checking on PAT/PMT tables due to broken packet rewriting hardware.
This is not a showstopper bug. It would simply be nice to fix before the release if the fix doesn't risk breaking anything. Feel free to reassign to me if this doesn't get fixed before release.
comment:8 Changed 18 years ago by
This is really showstopper bug for me every day. Backend runs max couble hours.
comment:9 Changed 18 years ago by
I've also started to have problems that crash the backend (1-2 times / day). Based on the logs, I believe it is this same problem (logs show PES packet related errors). If needed, I can get logs and backtraces and/or test patches.
comment:10 Changed 18 years ago by
Severity: | medium → high |
---|
looks like same problem according to the logs... PES packet related error... backend usualy dead after 2-3 hours of live tv....
comment:11 Changed 18 years ago by
Severity: | high → medium |
---|
comment:12 Changed 18 years ago by
Has to be said, I agree with the high severity. I'd consider myself luck if I get 2-3 hours before my backend dies as a result of this bug, certainly I'd say it's blocker for 0.20. Let me know if I can help. I have BT's but I don't think they look useful as to the root cause :(
comment:13 Changed 18 years ago by
Priority: | minor → major |
---|
We need valgrind logs of the backend built with Janne's pespacket_assembler_debug.diff to try to identify the cause of this.
The valgrind command line to use was recently pasted to the dev list
Changed 18 years ago by
Attachment: | 2147_fix.diff added |
---|
comment:14 Changed 18 years ago by
Type: | defect → patch |
---|
Changed 18 years ago by
Attachment: | drop_stuffing_tables.diff added |
---|
comment:15 Changed 18 years ago by
added patch to drop stuffing packet before the CRC check in HandleTSTables().
There is another one with -v siparser. That can't easily removed. The main problem is that we handle DVB SI tables as PES packets which is incorrect.
comment:16 Changed 18 years ago by
Tested last night - applied both patches. No crashes when recording two long movies. While I can't be sure, it's certainly seems to work for me :)
comment:17 Changed 18 years ago by
comment:18 Changed 18 years ago by
comment:19 Changed 18 years ago by
Tested the patches for one night, so far everything is working fine!
comment:20 Changed 18 years ago by
Milestone: | 0.20 → 0.21 |
---|---|
Priority: | major → minor |
There's stlll a small issue left, but it looks like the majority of problems have been fixed.
Excerpt from Mark B's valgrind logs.
==8502== Use of uninitialised value of size 4 ==8502== at 0x456BCAE: MPEGDescriptor::DescriptorLength() const (mpegdescriptors.h:145) ==8502== by 0x45ABED2: MPEGDescriptor::Parse(unsigned char const*, unsigned) (mpegdescriptors.cpp:18) ==8502== by 0x47CD2ED: EITHelper::AddEIT(DVBEventInformationTable const*) (eithelper.cpp:243) ==8502== by 0x45A09FC: DVBStreamData::HandleTables(unsigned, PSIPTable const&) (dvbstreamdata.cpp:290) ==8502== by 0x457FE80: MPEGStreamData::HandleTSTables(TSPacket const*) (mpegstreamdata.cpp:701) ==8502== by 0x4578BDA: MPEGStreamData::ProcessTSPacket(TSPacket const&) (mpegstreamdata.cpp:739) ==8502== by 0x4578CF2: MPEGStreamData::ProcessData(unsigned char*, int) (mpegstreamdata.cpp:724) ==8502== by 0x48AE23E: DVBSignalMonitor::RunTableMonitorTS() (dvbsignalmonitor.cpp:386) ==8502== by 0x48AEACE: DVBSignalMonitor::RunTableMonitor() (dvbsignalmonitor.cpp:563) ==8502== by 0x48AEAF8: DVBSignalMonitor::TableMonitorThread(void*) (dvbsignalmonitor.cpp:173) ==8502== by 0x608807C: start_thread (in /lib/tls/libpthread-2.3.6.so) ==8502== by 0x62728FD: clone (in /lib/tls/libc-2.3.6.so) ==8502==
comment:21 Changed 18 years ago by
Status: | new → assigned |
---|---|
Type: | patch → defect |
comment:22 follow-up: 23 Changed 18 years ago by
Hi, Backend still stops :(
myth.log 2006-09-12 07:09:34.660 TVRec(1): Got good signal 2006-09-12 07:09:34.664 TVRec(1): ClearFlags?(WaitingForSignal?,) -> RunMainLoop?,AskAllowRecording?,SignalMonitorRunning?,EITScannerRunning, 2006-09-12 07:09:34.700 DVBSM(0)::AddPIDFilter(0x12): 2006-09-12 07:09:35.122 DTVSM(0)::SetNIT(): net_id = 100 2006-09-12 07:09:35.123 SM(0)::AddFlags?: Seen(NIT,) Match() Wait() 2006-09-12 07:09:35.124 DTVSM(0)::SetNIT(): net_id = 100 2006-09-12 07:09:35.126 SM(0)::AddFlags?: Seen(NIT,) Match() Wait() 2006-09-12 07:09:36.166 match[0]: 0 vs. 2006-09-12 07:09:36.618 match[0]: 0 vs. 2006-09-12 07:10:01.942 PESPacket: Failed CRC check 0x0 != 0xbc12eeb4 for StreamID = 0x0 2006-09-12 07:10:01.948 PESPacket: Failed CRC check 0x0 != 0xbc12eeb4 for StreamID = 0x0 2006-09-12 07:10:01.949 PESPacket: Failed CRC check 0x0 != 0xbc12eeb4 for StreamID = 0x0 2006-09-12 07:10:01.951 PSIP packet failed CRC check. pid(0x0) type(0x0) 2006-09-12 07:10:01.954 SM(1)::AddFlags?: Seen(PAT,) Match() Wait() 2006-09-12 07:10:01.956 SM(1)::AddFlags?: Seen() Match(PAT,) Wait() 2006-09-12 07:10:01.957 CreatePATSingleProgram() 2006-09-12 07:10:01.958 PAT in input stream
gdb.txt is attached.
comment:23 Changed 18 years ago by
Replying to petri@heinala.fi:
Hi, Backend still stops :(
gdb.txt is attached.
you've managed to delete the few crucial lines where gdb tells us how and where it crashed. Please compress the gdb.txt file and re-upload it.
comment:25 Changed 18 years ago by
that error is known. you can work around it by running mythbackend with the default verbose options.
comment:26 Changed 17 years ago by
comment:27 Changed 17 years ago by
comment:28 Changed 17 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
probably fixed by [12480]
Issue not entirely solved. valgrind reports following error
and I see sometimes a bunch of "AddTSPacket: Out of sync!!! Need to wait for next payloadStart PID: 0x12, continuity counter: 6 (expected 5)."
The continuity counter is always expected+1.