[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:
>>>> 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.

Any suggestions?  I don't expect step-by-step instructions.  A pointer 
or two should be enough as long as it doesn't entail a re-write of mpeg.c :)

Richard.