[FFmpeg-devel] [PATCH] Add SUP/PGS subtitle demuxer

[Hendrik Leppkes]

On Sun, Aug 31, 2014 at 9:41 AM, Reimar Döffinger
<Reimar.Doeffinger at gmx.de> wrote:
> On 30.08.2014, at 15:38, wm4 <nfxjfg at googlemail.com> wrote:
>> +    // The packet-size is stored as part of the packet.
>> +    if ((ret = avio_read(s->pb, tmp, 3)) < 0)
>> +        return ret;
>> +
>> +    len = AV_RB16(tmp + 1);
>> +
>> +    if ((ret = av_new_packet(pkt, len + 3)) < 0)
>> +        return ret;
>> +
>> +    memcpy(pkt->data, tmp, 3);
>> +
>> +    if ((ret = avio_read(s->pb, pkt->data + 3, len)) < 0) {
>> +        av_free_packet(pkt);
>> +        return ret;
>> +    }
>
> I think this will not handle short reads correctly, retuning uninitialised data.
> My suggestion would be to read the length, then seek back (buffering should ensure this is no issue even if we read from stdin) and then use the functions to read the full packet with all the proper error handling.


I don't see a problem in the code he proposed. It seems to handle
every read with an error check, without relying on potential buffering
of data to allow a seek backwards.
This is assuming avio_read does return an error if it runs against
EOF, which I assume it does? I didn't double check that.

What exactly do you think is wrong here?

- Hendrik