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

[Michael Niedermayer]

On Sat, Apr 25, 2020 at 01:10:53PM +0000, Zane van Iperen wrote:
> On Sat, 25 Apr 2020 13:15:25 +0200
> "Michael Niedermayer" <michael at niedermayer.cc> wrote:
> 
> > > +static int pp_bnk_probe(const AVProbeData *p)
> > > +{
> > > +    uint32_t sample_rate = AV_RL32(p->buf +  4);
> > > +    uint32_t track_count = AV_RL32(p->buf + 12);
> > > +    uint32_t flags       = AV_RL32(p->buf + 16);
> > > +
> > > +    if (track_count == 0 || sample_rate == 0)
> > > +        return 0;  
> > 
> > the header code checks these also for INT_MAX
> > 
> > 
> 
> See below I check both track_count and sample_rate for sane upper
> limits. Is it worth adding an INT_MAX check too?

why not ? its rejected by the header reading code


> 
> 
>     /* These limits are based on analysing the game files. */
>     if (track_count > 113)
>         return 10;
> 
>     if ((sample_rate !=  5512) && (sample_rate != 11025) &&
>         (sample_rate != 22050) && (sample_rate != 44100))
>         return 10;
> 
> 
> 
> > > +
> > > +    /* Sometimes we have the first track header, so check that
> > > too. */
> > > +    if (p->buf_size >= 32 && AV_RL32(p->buf + 28) != sample_rate)
> > > +        return 0;  
> > 
> > are files with 32 or less bytes valid ?
> > If not its probably better to not recognize such small pieces
> > 
> 
> A file needs at least one track and the first track header is
> immediately after the file header, so I guess the minimum valid size
> would be 40 (assuming an empty track).
> 
> Would removing the "p->buf_size >= 32" check be alright?

yes, that should be ok due to AVPROBE_PADDING_SIZE

also iam not sure it makes sense or if it goes to far but the probe
code could check all tracks which are within the buffer
all these low 10 scores are a bit ugly, some more solid "yes iam sure
this is / is not" instead of such really low scores.

low scores which get returned based on few weak checks have the problem
of more often producing errors in detection. Random non multimedia files being
detected as something they are not or if 2 demuxers have such a low score
detection then its more likely the wrong one will be choosen


thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200426/9d4943c8/attachment.sig>