[FFmpeg-devel] [PATCH v3 3/7] lavf: APV demuxer

[Mark Thompson]

On 24/04/2025 01:10, James Almer wrote:
> On 4/23/2025 5:45 PM, Mark Thompson wrote:
>> +static int apv_read_header(AVFormatContext *s)
>> +{
>> +    AVStream *st;
>> +    GetByteContext gbc;
>> +    APVHeaderInfo header;
>> +    uint8_t buffer[28];
>> +    uint32_t au_size, signature, pbu_size;
>> +    int err, size;
>> +
>> +    err = ffio_ensure_seekback(s->pb, sizeof(buffer));
> 
> Isn't 28 bytes small enough that a backwards avio_seek() should always succeed?

I don't see any documentation to that effect and I'm not familiar with the details of the implementation.

There are various calls in other places in lavf which give single-digit numbers to ffio_ensure_seekback, so I'll keep it unless there is a guarantee somewhere that I've missed.

>> +    if (err < 0)
>> +        return err;
>> +    size = avio_read(s->pb, buffer, sizeof(buffer));
>> +    if (size < 0)
>> +        return size;
>> +
>> +    bytestream2_init(&gbc, buffer, sizeof(buffer));
>> +
>> +    au_size = bytestream2_get_be32(&gbc);
>> +    if (au_size < 24) {
>> +        // Too small.
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +    signature = bytestream2_get_be32(&gbc);
>> +    if (signature != APV_SIGNATURE) {
>> +        // Signature is mandatory.
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +    pbu_size = bytestream2_get_be32(&gbc);
>> +    if (pbu_size < 16) {
>> +        // Too small.
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +
>> +    err = apv_extract_header_info(&header, &gbc);
>> +    if (err < 0)
>> +        return err;
>> +
>> +    st = avformat_new_stream(s, NULL);
>> +    if (!st)
>> +        return AVERROR(ENOMEM);
>> +
>> +    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>> +    st->codecpar->codec_id   = AV_CODEC_ID_APV;
>> +    st->codecpar->format     = header.pixel_format;
>> +    st->codecpar->profile    = header.profile_idc;
>> +    st->codecpar->level      = header.level_idc;
>> +    st->codecpar->width      = header.frame_width;
>> +    st->codecpar->height     = header.frame_height;
>> +
>> +    st->avg_frame_rate = (AVRational){ 30, 1 };
>> +    avpriv_set_pts_info(st, 64, 1, 30);
>> +
>> +    avio_seek(s->pb, -size, SEEK_CUR);
>> +
>> +    return 0;
>> +}
>> +
>> +static int apv_read_packet(AVFormatContext *s, AVPacket *pkt)
>> +{
>> +    uint32_t au_size;
>> +    int ret;
>> +
>> +    au_size = avio_rb32(s->pb);
>> +    if (au_size == 0 && avio_feof(s->pb))
>> +        return AVERROR_EOF;
>> +    if (au_size < 16 || au_size > 1 << 24) {
> 
> Might be a good idea to also check for the signature.

Fair, added.

Also made the upper limit a bit bigger because it feels a little too close if 16K video is ever a thing.

>> +        av_log(s, AV_LOG_ERROR, "APV AU is bad\n");
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +
>> +    ret = av_get_packet(s->pb, pkt, au_size);
>> +    pkt->flags        = AV_PKT_FLAG_KEY;
>> +
>> +    return ret;
>> +}

Thanks,

- Mark