[MPlayer-dev-eng] Re: AC-3 in a TS stream

[Nico Sabbi]

Trent Piepho wrote:

>http://www.speakeasy.org/~xyzzy/download/ktwb-[1259].ts.bz2
>  
>

downloaded, thanks

>1 & 2 don't work (all detected private1), 5 & 9 do (ac3 DVD VOB (incorrectly)
>detected).
>
>I think there might be a potential bug in the pes_parse2() code.  You know
>this I'm sure, but I'll say it anyway.  All kinds of PES packets have a 6 byte
>header.  Most PES packets, but not all, have a 3+ byte header after that.  If
>the stream_id is for example private2 or padding, the second 3+ byte header
>isn't present.  The code in pes_parse2() doesn't check stream_id, and will
>always try to parse the optional 3+ byte header, even when it doesn't exist.
>
>I think all that can go wrong is the ts_demux might spit out bogus error
>messages, and it will mangle that PES stream.  mplayer can't do anything with
>private2 or the other types, so it hardly matters that the data is mangled.
>
>  
>

stuffing and pes2 aren't used, so it's not a problem

>>I guess I know why: other initial bytes than 0x8n in the next payload 
>>packet will make
>>demux_ts forget that previously it found a PES_PRIVATE1 stream
>>    
>>
>
>Hmm, why do you think this?  I thought the only check for 0x8n was inside
>pes_parse2().  pes_parse2() is only called for the first TS packet in a PES
>packet, when is_start is 1.  When is_start is zero pes_parse2() is never
>called and there isn't any kind of check for 0x8n in the payload.
>
>It seems pretty clear to me from the log I posted that only the first TS
>packet of a PES packet (when is_start==1) is sent to a52_check().  The rest
>of the TS packets in the PES packet (when is_start==0) are just discarded.
>  
>

if pes_parse2() first finds a PES_PRIVATE1 stream and next a AUDIO_A52 
with header,
the (else if ((p[0] & 0xF8) == 0x80)) then the first PES_PRIVATE1 will 
be forgotten.

>I think the problem might be in ts_parse().  When in probe mode
>tss->payload_size is never set to es->payload_size.  This means ts_parse()
>just skips is_start==0 TS packets and doesn't add them to the PES packet and
>so a52_check will never see them.  At least this appears to fix the
>problem for me.
>  
>

I have to verify

>  
>
>>>Anyway, I looked at how mplex and dvdauthor deal with DVD VOB AC3 sub-headers,
>>>since there is no spec I could find.  This is what I found:
>>>
>>>dvdauthor will only look for a sub-header if the PTS_DTS flag is set in the
>>>PES header.
>>>
>>>      
>>>
>>the sub header is always needed, even with no pts/dts
>>    
>>
>
>So what you are saying is that you have seen TS streams where the AC3 PES
>packets had a DVD VOB sub-header but the pts/dts flag in the PES header
>_wasn't_ set?  dvdauthor would not accept this as valid, which doesn't prove
>such streams do not exist of course.
>  
>

yes, sometimes. BTW, dvdauthor doesn't accept TS streams

>
>  
>
>>>dvdauthor and mplex expect the AC3 substream ID to be, in binary, 10000xxx,
>>>for AC-3 substream xxx.  The code for a DTS substream is 10001xxx.  The TS
>>>auto-detect code checks for 1000xxxx, one bit less specific than it could be.
>>> 
>>>
>>>      
>>>
>>of course: it searches AC3.
>>0x80 -> 0x87 : AC3
>>0x88 -> 0x8f : DTS (that I don't want to deal with)
>>    
>>
>
>I'm not sure I understand you here.  The code I see in demux_ts.c is this:
>
>                else if ((p[0] & 0xF0) == 0x80)
>
>This is clearly detecting both 0x80 -> 0x87 and 0x88 ->0x8f as AC3.  You are
>saying you want to detect DTS as AC3, rather than ignore it or deal with as
>DTS?  Or do you mean there is a small bug here, and it should be p[0] & 0xf8
>that is checked?  The latter it what I was trying to suggest in my previous
>email.
>  
>

yes, bug. Fixed locally

>>no, it's the count of AC3 frames present in the current pes packet
>>    
>>
>
>Ok, that makes sense.  Couldn't this be checked?  The code knows the size of
>the PES packet and AC3 frames have a limited range of valid sizes (128 to 3840
>bytes).  For example, a PES with 5KB of payload can only have 1 - 40 AC3
>syncwords.  So:
>
>if(p[1]*128-127 >= es->payload_size) { not DVD VOB format }
>
>this would cause fewer false positives of DVD VOB format, but not mess up
>detection of real DVD VOB format packets, right?
>  
>

that frame_count is totally unreliable: sometimes it's missing, 
sometimes it's fake.
There are easier ways to check the presence of ac3

>  
>
>>>The next two bytes of the sub-stream header are an offset to the start of an
>>>AC3 frame.  If the offset is 1 (I think, maybe 2?), then the AC3 frame
>>>syncword must start immediately after the sub-stream header.  This is what
>>>I suggested before, and what your patch checks for.
>>>      
>>>
>>yes
>>    
>>
>
>If the offset is 0, does that mean no ac3 frames start in the packet? 
>

in theory it means that no ac3 frame starts in this pes, in practice it 
may indicate
a broken muxer

...

> Or does
>it mean the start location just isn't specified and an AC3 frame may or may
>not start in the packet?  Could the offset extend beyond the *PES* packet and
>into the next PES packet? 
>
>
>Maybe this has changed in the most recent CVS?  In MPlayer-1.0pre7try2:
>
>#define TS_MAX_PROBE_SIZE 2000000 /* dont forget to change this in cfg-common.h too */
>off_t ts_probe = TS_MAX_PROBE_SIZE;
>
>I checked that running mplayer without -tsprobe results in the same number of
>TS packets getting scanned as with -tsprobe 2000000, while -tsprobe 32768
>scans far fewer packets.  32KB is of course far too small for 40 Mbit QAM-256
>streams.  Maybe the ts probe max should be increased?  If the PAT and PMT are
>sent at the minimum rate, 32KB isn't likely to get them.  It would be even
>worse in PAL countries, where QAM-256 can fit something like 52 Mbit/sec in an
>8 MHz channel.
>
>
>  
>

sorry, I forgot it (too long time has gone): the CHECK size is 64 KB: 
it's the size
used to verify that the file is a TS; the PROBE size is ~2 MB (interval 
used to
analyze the streams).
A too large probing size will slow down startup time a lot, so I prefer 
that the user
specify it explicitly.

I almost forgot in what a  bad shape I left some (and large) parts of 
the code.
This demuxer needs a major reorganization and simplification. I also 
have in mind
a safer way to handle with these ac3 streams.
It will take some time, so don't hold your breath.

Bye,

    Nico