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

[Richard]

On 03/03/13 16:09, Richard wrote:
> 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.

And another that hopefully successfully addresses the negative seek 
issue.  Once a DVD stream has been determined, no more packet inspection 
is performed, so any seeks will only happen once, when the first 
PRIVATE_STREAM_2 packet is detected.

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: 10744 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130308/5bb3d892/attachment.bin>