[FFmpeg-devel] [PATCH] Demuxer for Leitch/Harris' VR native stream format (LXF)

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Mon, Sep 13, 2010 at 10:56:43AM +0100, M?ns Rullg?rd wrote:
>> Tomas H?rdin <tomas.hardin at codemill.se> writes:
> [...]
>> >> > +//reads and checksums packet header. returns format and size of payload
>> >> > +static int get_packet_header(AVFormatContext *s, unsigned char *header,
>> >> > +                             uint32_t *format)
>> >> > +{
>> >> > +    LXFDemuxContext *lxf = s->priv_data;
>> >> > +    ByteIOContext   *pb  = s->pb;
>> >> > +    int size, track_size, samples;
>> >> > +    AVStream *st;
>> >> > +
>> >> > +    if (get_buffer(pb, header, LXF_PACKET_HEADER_SIZE) != LXF_PACKET_HEADER_SIZE)
>> >> > +        return AVERROR(EIO);
>> >> > +
>> >> > +    if (memcmp(header, LXF_IDENT, LXF_IDENT_LENGTH)) {
>> >> > +        av_log(s, AV_LOG_ERROR, "packet ident mismatch - out of sync?\n");
>> >> > +        return -1;
>> >> > +    }
>> >> > +
>> >> > +    if (check_checksum(header))
>> >> > +        av_log(s, AV_LOG_ERROR, "checksum error\n");
>> >> > +
>> >> > +    *format = AV_RL32(&header[32]);
>> >> > +    size    = AV_RL32(&header[36]);
>> >> > +
>> >> > +    //type
>> >> > +    switch (AV_RL32(&header[16])) {
>> >> > +    case 0:
>> >> > +        //video
>> >> > +        //skip VBI data and metadata
>> >> > +        url_fskip(pb, AV_RL32(&header[44]) + AV_RL32(&header[52]));
>> >> 
>> >> cant this lead to a backward seek and thus infinite loop ?
>> >
>> > Yes, assuming AV_RL32() returns signed. Fixed by casting to uint32_t and
>> > splitting up into two skips.
>
> split is ugly
>
>> 
>> AV_RL32() returns an unsigned value.
>
> maybe it should but i dont think it does
>
> #   define AV_RL32(x)                           \
>     ((((const uint8_t*)(x))[3] << 24) |         \
>      (((const uint8_t*)(x))[2] << 16) |         \
>      (((const uint8_t*)(x))[1] <<  8) |         \
>       ((const uint8_t*)(x))[0])

You are right.  Should we change it?

-- 
M?ns Rullg?rd
mans at mansr.com