[FFmpeg-devel] [RFC/PATCH] Pass PRIVATE_STREAM_2 MPEG-PS packets to caller

[Richard]

On 15/03/13 00:10, Michael Niedermayer wrote:
> On Fri, Mar 08, 2013 at 09:41:50PM +0100, Richard wrote:
>>       if (startcode == PRIVATE_STREAM_2) {
>> -        len = avio_rb16(s->pb);
>> -        if (!m->sofdec) {
>> -            while (len-- >= 6) {
>> +        if (!m->sofdec && !m->dvd) {
>> +            int origlen = len = avio_rb16(s->pb);
>> +            uint8_t firstbyte = avio_r8(s->pb);
>> +            /* 'Unread' the first byte so we can search the whole
>> +             * buffer for 'Sofdec'.  Skipping back 1 byte should
>> +             * never incur a physical read as even with a 1 byte buffer
>> +             * the 2nd byte wouldn't be physically read until requested.
>> +             */
>
>> +            avio_skip(s->pb, -1);
>
> an av_assert0() is needed, here, i dont want to have to debug such
> failures

Ok, added.

>> +
>> +            while (len >= 6) {
>> +                len--;
>>                   if (avio_r8(s->pb) == 'S') {
>>                       uint8_t buf[5];
>>                       avio_read(s->pb, buf, sizeof(buf));
>> @@ -259,9 +269,32 @@ static int mpegps_read_pes_header(AVFormatContext *s,
>>                   }
>>               }
>>               m->sofdec -= !m->sofdec;
>
> you change sofdec detection behavior here
> before it run once, after your patch it runs on every packet

No it doesn't.  There's still the check for '!m->sofdec', it's just been 
extended to also check for '!m->dvd'.  Once the first packet has been 
checked, m->sofdec is either 1 or -1, so that code will only ever be 
executed once.

>> +
>> +            if (m->sofdec <= 0 &&
>> +                ((origlen == 980  && firstbyte == 0) ||
>> +                 (origlen == 1018 && firstbyte == 1))) {
>> +                /* DVD NAV packet, move back to the start of the stream (plus 'length' field) */
>> +                m->dvd = 1;
>> +                if (avio_skip(s->pb, -((origlen-len) + 2)) < 0) {
>
> if you do seek back anyway then you could check more than 1 byte of
> the packet

To be honest, there's not much else that can be checked.  The PCI packet 
(firstbyte == 0) has the two fields vobu_s_ptm and vobu_e_ptm, which 
could be checked to ensure vobu_e_ptm > vobu_s_ptm but that's about it. 
  The DSI packet (firstbyte == 1) doesn't really have anything that can 
be easily used as an identifier.

To my mind, changing the code to be able to check the two fields in the 
PCI packet would be a lot of effort for very little gain.

Attached is a patch that adds the requested assertion.

Richard.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-passing-DVD-navigation-packets-startcode-0x1bf-t.patch
Type: text/x-patch
Size: 11615 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130315/fe8fc719/attachment.bin>