[FFmpeg-devel] [PATCH v7 2/2] avformat: add demuxer for Pro Pinball Series' Soundbanks

[Zane van Iperen]

On Mon, 6 Apr 2020 17:39:13 +0200
"Andreas Rheinhardt" <andreas.rheinhardt at gmail.com> wrote:

> > +    /* Parse and validate each track. */
> > +    for (int i = 0; i < hdr.track_count; i++) {
> > +        PPBnkTrack e;
> > +
> > +        if ((ret = avio_read(s->pb, buf, PP_BNK_TRACK_SIZE)) < 0) {
> > +            goto done;
> > +        } else if (ret != PP_BNK_TRACK_SIZE) {
> > +            ret = AVERROR(EIO);
> > +            goto done;
> > +        }  
> 
> If your file is truncated and missing the data as well as the header
> of one of the latter tracks, you will ignore everything in the file
> even if the earlier tracks are completely there. Instead you could
> reset track_count to the number of tracks for which data is available
> and only error out if that number is zero.

Have done this.


> > +
> > +    ret = 0;
> > +  
> You could just return 0 here and convert your done into a fail. This
> would make the intent clearer.


Done.

> > +    size = FFMIN(trk->data_size - ctx->bytes_read,
> > PP_BNK_MAX_READ_SIZE); +
> > +    if ((ret = av_get_packet(s->pb, pkt, size)) < 0)
> > +        return ret;
> > +    else if (ret != size)
> > +        return AVERROR_INVALIDDATA;
> > +
> > +    ctx->bytes_read    += ret;  
> 
> If you have read something, but not enough, your internal counter will
> be out of sync with the file. This doesn't seem to be a problem, so I
> don't know if something should be done about it.

Fixed it anyway, changed to:

    if ((ret = av_get_packet(s->pb, pkt, size)) < 0)
        return ret;

    ctx->bytes_read    += ret;
    pkt->flags         &= ~AV_PKT_FLAG_CORRUPT;
    pkt->stream_index   = ctx->current_track;
    pkt->duration       = ret * 2;

Zane