[pvrusb2] xawtv fix

[Mike Isely]

On Tue, 11 Apr 2006, xavier.gnata at free.fr wrote:

> Hi,
>
> Thanks for this long explanation.
> I'm not mpeg aware engough to solve this problem :(
> You should ask guys on mplayer devel list. If you do so, could you post a
> reference to your question on this list?
>
> Xavier
>

Well it turns out my explanation was wrong....

Short summary: I now have xawtv working regardless of the setting in the 
driver.  This looks like it might be a bug in xawtv, or at least fragile 
behavior when trying to locate packet headers.

Longer story:

I did some reading tonight about the mpeg2 standard and quickly realized 
that it was simply impossible for mplayer to still work with that 0xc0 
packet type missing.  I then looked (closer) at dvbsnoop output of a 
sample stream I had captured Sunday and sure enough the 0xc0 type was 
present.  No idea why I missed it before - perhaps I only saw then what I 
wanted to see.  Just to be sure, I whipped up a quickie C program to scan 
an mpeg2 stream and printf() every single packet header it could find and 
there are indeed a whole bunch of type 0xc0 packets present.  So the 
stream data is fine.

So that brings back the question for why xawtv could be sensitive to this? 
I had previously concluded that the mpeg parser in xawtv wasn't finding 
the 0xc0 type because it just wasn't there, but now knowing that it was 
present I took a closer look at the parser, and found several other 
reasons why it could fail to find that type.  One of the reasons was an 
apparent loss of synchronization internal to the parser.  There's a fetch 
function inside the parser that tries to walk the stream of packets header 
by header, but if it fails to see a valid header it gives up.  I stuffed a 
printf() there, and found that indeed this was the condition killing it. 
So I modified this function to slide forward one byte and repeat the 
check.  With that change in place, xawtv starts working again.  If you 
would like to try this, edit parse-mpeg.c, search for the function 
mpeg_find_ps_packet() and change this code:

     /* read header */
     for (;;) {
 	buf = mpeg_get_data(h,*pos,16);
 	if (NULL == buf)
 	    return 0;
 	if (buf[0] != 0x00 ||
 	    buf[1] != 0x00 ||
 	    buf[2] != 0x01)
 	    return 0;

to this:

     /* read header */
     for (;;) {
 	buf = mpeg_get_data(h,*pos,16);
 	if (NULL == buf)
 	    return 0;
 	if (buf[0] != 0x00 ||
 	    buf[1] != 0x00 ||
 	    buf[2] != 0x01) {
             if ((*pos - start) < FILE_BUF_MIN) {
                 *pos += 1;
                 continue;
             }
 	    return 0;
         }

The buf[0] / buf[1] / buf[2] check is an attempt to verify that it is 
looking at the beginning of a packet.  This function seems to be assuming 
that it is always looking at the front of a packet.  I'm not convinced 
that such an assumption is really correct.  The change causes it to try 
again one byte further if there isn't a match - previously it just quit.

One could argue that this sort of problem should only be possible if there 
is garbage data interspersed in the mpeg2 stream.  That might be true, and 
I don't know if the mpeg2 standard would allow for that.  But an mpeg 
parser should probably be robust against synchronization errors in any 
case.  That's what this change does.

Another thing that makes me suspicious of xawtv is that the area of code 
which is involved with this (mpeg_get_data()) is a hairy complex thing 
which tries to dynamically size a read buffer with some kind of sliding 
window capability - and I think that same area of code has been 
responsible for the 'zero byte read' misbehavior I've seen in the past. 
I don't yet follow how that function is actually working.  Suffice to say 
that it is a messy piece of code so this problem could just be uncovering 
another dormant bug there.  As for why that driver changed tickled this 
problem?  I'm not entirely sure.  Maybe with that second cx23416 command 
argument set to 2 that the cx23416 is multiplexing in some other kind of 
data type or maybe it's injecting filler bytes that it didn't have to do 
before.  Or maybe the whole thing is just a question of timing and we've 
just been lucky until now.  (Note I recently found and fixed a nasty race 
condition / bug in pvrusb2 that's been since the beginning, lying dormant, 
so I certainly wouldn't accuse xawtv of having a monopoly on dormant 
bugs!)

I'll see about contacting the xawtv maintainer.

   -Mike

-- 
                         |         Mike Isely          |     PGP fingerprint
      Spammers Die!!     |                             | 03 54 43 4D 75 E5 CC 92
                         |   isely @ pobox (dot) com   | 71 16 01 E2 B5 F5 C1 E8
                         |                             |