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

[Richard]

On 03/03/13 10:25, Richard wrote:
> On 03/03/13 01:55, Michael Niedermayer wrote:
>> On Sun, Mar 03, 2013 at 12:51:46AM +0100, Richard wrote:
>>> On 02/03/13 13:18, Michael Niedermayer wrote:
>> [...]
>>> +static int dvd_nav_parse(AVCodecParserContext *s,
>>> +                         AVCodecContext *avctx,
>>> +                         const uint8_t **poutbuf, int *poutbuf_size,
>>> +                         const uint8_t *buf, int buf_size)
>>> +{
>>> +    DVDNavParseContext *pc1 = s->priv_data;
>>> +    ParseContext *pc        = &pc1->pc;
>>> +    int lastPacket          = 0;
>>> +    int valid               = 0;
>>> +
>>> +    s->pict_type = AV_PICTURE_TYPE_NONE;
>>> +
>>
>>> +    avctx->time_base.num = 1;
>>> +    avctx->time_base.den = 90000;
>>
>> why is this needed ?
>
> Because otherwise the AVPacket returned to the calling application ends
> up with a duration of 0.
>
> <snip>
>
>>> +        }
>>> +    }
>>> +
>>> +    if (!valid || lastPacket) {
>>> +        pc1->copied = 0;
>>> +        pc1->lba    = 0xFFFFFFFF;
>>> +    }
>>
>> the {} placement in this file is rather inconsistent
>
> Yes, you're right.  Sorry, I was trying to keep to K&R style, which
> seems to be the main style used, but which isn't my normal style.  I'll
> tidy that up.
>
> <snip>
>
>>> diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
>>> index 4eaffd8..98ddc88 100644
>>> --- a/libavformat/mpeg.c
>>> +++ b/libavformat/mpeg.c
>>> @@ -247,9 +247,12 @@ static int
>>> mpegps_read_pes_header(AVFormatContext *s,
>>>           goto redo;
>>>       }
>>>       if (startcode == PRIVATE_STREAM_2) {
>>> -        len = avio_rb16(s->pb);
>>> +        int origlen = len = avio_rb16(s->pb);
>>> +        uint8_t firstbyte = avio_r8(s->pb);
>>> +        avio_skip(s->pb, -1);
>>>           if (!m->sofdec) {
>>> -            while (len-- >= 6) {
>>> +            while (len >= 6) {
>>> +                len--;
>>>                   if (avio_r8(s->pb) == 'S') {
>>>                       uint8_t buf[5];
>>>                       avio_read(s->pb, buf, sizeof(buf));
>>> @@ -260,8 +263,15 @@ static int
>>> mpegps_read_pes_header(AVFormatContext *s,
>>>               }
>>>               m->sofdec -= !m->sofdec;
>>>           }
>>> -        avio_skip(s->pb, len);
>>> -        goto redo;
>>> +        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) */
>>
>>> +            avio_skip(s->pb, -((origlen-len) + 2));
>>
>> this seeks backward which is not guranteed to work on non seekable
>> protocols. On a dvd it would work but might if the OS cannot satisfy
>> it from some cache do an actual seek which is slow.
>> the same issue exists a fiw lines above
>
> Yes, I wasn't completely happy with that myself.  The problem is that
> the contents need to be inspected to determine the codec for the packet
> (in the case of a DVD) or the codec to use for audio (in the case of
> Sofdec).  But inspecting the contents here prevents them being passed on
> without a negative seek.  How should/can that be solved here?

Ok, attached is a patch that addresses the formatting/code style issues.

Regarding the negative seek, I see that a negative seek is also 
performed on each video packet:

     } else if (startcode >= 0x1e0 && startcode <= 0x1ef) {
         static const unsigned char avs_seqh[4] = { 0, 0, 1, 0xb0 };
         unsigned char buf[8];
         avio_read(s->pb, buf, 8);
         avio_seek(s->pb, -8, SEEK_CUR);
         if(!memcmp(buf, avs_seqh, 4) && (buf[6] != 0 || buf[7] != 1))
             codec_id = AV_CODEC_ID_CAVS;
         else
             request_probe= 1;
         type = AVMEDIA_TYPE_VIDEO;


That's not to say that two wrongs make a right but that's almost 
certainly where I saw that negative seeks are possible and work.

I'm happy to change this if you have any suggestions how to solve this 
without the negative seek.

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: 10230 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130303/f40806e3/attachment.bin>