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

Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
---
 Changelog                |   1 +
 libavformat/Makefile     |   1 +
 libavformat/allformats.c |   1 +
 libavformat/pp_bnk.c     | 218 +++++++++++++++++++++++++++++++++++++++
 libavformat/version.h    |   2 +-
 5 files changed, 222 insertions(+), 1 deletion(-)
 create mode 100644 libavformat/pp_bnk.c

Am Mi., 18. März 2020 um 15:12 Uhr schrieb Zane van Iperen
<zane@zanevaniperen.com>:
>
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
>  Changelog                |   1 +
>  libavformat/Makefile     |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/pp_bnk.c     | 218 +++++++++++++++++++++++++++++++++++++++
>  libavformat/version.h    |   2 +-
>  5 files changed, 222 insertions(+), 1 deletion(-)
>  create mode 100644 libavformat/pp_bnk.c
>
> diff --git a/Changelog b/Changelog
> index 927dc89c51..0970bfb11e 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -55,6 +55,7 @@ version <next>:
>  - CRI HCA decoder
>  - CRI HCA demuxer
>  - Cunning Developments ADPCM decoder
> +- Pro Pinball Series Soundbank demuxer

Any restrictions on track_count and sample_rate that can be used for
auto-detection?

Not a necessity of course...

Carl Eugen

On Wed, 18 Mar 2020 15:19:19 +0100
"Carl Eugen Hoyos" <ceffmpeg@gmail.com> wrote:

> Am Mi., 18. März 2020 um 15:12 Uhr schrieb Zane van Iperen
> <zane@zanevaniperen.com>:
> >
> > Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> > ---
> >  Changelog                |   1 +
> >  libavformat/Makefile     |   1 +
> >  libavformat/allformats.c |   1 +
> >  libavformat/pp_bnk.c     | 218
> > +++++++++++++++++++++++++++++++++++++++ libavformat/version.h
> > |   2 +- 5 files changed, 222 insertions(+), 1 deletion(-)
> >  create mode 100644 libavformat/pp_bnk.c
> >
> > diff --git a/Changelog b/Changelog
> > index 927dc89c51..0970bfb11e 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -55,6 +55,7 @@ version <next>:
> >  - CRI HCA decoder
> >  - CRI HCA demuxer
> >  - Cunning Developments ADPCM decoder
> > +- Pro Pinball Series Soundbank demuxer  
> 
> Any restrictions on track_count and sample_rate that can be used for
> auto-detection?
> 

Not really :/

The largest sample rate I've seen is 44100, but I'd be hesitant to
probe based on that alone...

> Not a necessity of course...
> 

> Carl Eugen

Zane

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Am Mi., 18. März 2020 um 15:30 Uhr schrieb Zane van Iperen
<zane@zanevaniperen.com>:
>
> On Wed, 18 Mar 2020 15:19:19 +0100
> "Carl Eugen Hoyos" <ceffmpeg@gmail.com> wrote:
>
> > Am Mi., 18. März 2020 um 15:12 Uhr schrieb Zane van Iperen
> > <zane@zanevaniperen.com>:
> > >
> > > Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> > > ---
> > >  Changelog                |   1 +
> > >  libavformat/Makefile     |   1 +
> > >  libavformat/allformats.c |   1 +
> > >  libavformat/pp_bnk.c     | 218
> > > +++++++++++++++++++++++++++++++++++++++ libavformat/version.h
> > > |   2 +- 5 files changed, 222 insertions(+), 1 deletion(-)
> > >  create mode 100644 libavformat/pp_bnk.c
> > >
> > > diff --git a/Changelog b/Changelog
> > > index 927dc89c51..0970bfb11e 100644
> > > --- a/Changelog
> > > +++ b/Changelog
> > > @@ -55,6 +55,7 @@ version <next>:
> > >  - CRI HCA decoder
> > >  - CRI HCA demuxer
> > >  - Cunning Developments ADPCM decoder
> > > +- Pro Pinball Series Soundbank demuxer
> >
> > Any restrictions on track_count and sample_rate that can be used for
> > auto-detection?
> >
>
> Not really :/
>
> The largest sample rate I've seen is 44100, but I'd be hesitant to
> probe based on that alone...

But for all tracks, the sample rate has to be identical?

How many tracks are there?

Carl Eugen

On Wed, 18 Mar 2020 15:33:47 +0100
"Carl Eugen Hoyos" <ceffmpeg@gmail.com> wrote:

> Am Mi., 18. März 2020 um 15:30 Uhr schrieb Zane van Iperen
> <zane@zanevaniperen.com>:
> >
> > On Wed, 18 Mar 2020 15:19:19 +0100
> > "Carl Eugen Hoyos" <ceffmpeg@gmail.com> wrote:
> >  
> > > Am Mi., 18. März 2020 um 15:12 Uhr schrieb Zane van Iperen
> > > <zane@zanevaniperen.com>:  
> > > >
> > > > Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> > > > ---
> > > >  Changelog                |   1 +
> > > >  libavformat/Makefile     |   1 +
> > > >  libavformat/allformats.c |   1 +
> > > >  libavformat/pp_bnk.c     | 218
> > > > +++++++++++++++++++++++++++++++++++++++ libavformat/version.h
> > > > |   2 +- 5 files changed, 222 insertions(+), 1 deletion(-)
> > > >  create mode 100644 libavformat/pp_bnk.c
> > > >
> > > > diff --git a/Changelog b/Changelog
> > > > index 927dc89c51..0970bfb11e 100644
> > > > --- a/Changelog
> > > > +++ b/Changelog
> > > > @@ -55,6 +55,7 @@ version <next>:
> > > >  - CRI HCA decoder
> > > >  - CRI HCA demuxer
> > > >  - Cunning Developments ADPCM decoder
> > > > +- Pro Pinball Series Soundbank demuxer  
> > >
> > > Any restrictions on track_count and sample_rate that can be used
> > > for auto-detection?
> > >  
> >
> > Not really :/
> >
> > The largest sample rate I've seen is 44100, but I'd be hesitant to
> > probe based on that alone...  
> 
> But for all tracks, the sample rate has to be identical?
> 

Yes, in each soundbank, all tracks must have the same sample rate as
what's in the initial header.

> How many tracks are there?
> 

The file with the most tracks I can find has 113.

> Carl Eugen

Zane

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Zane van Iperen:
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
>  Changelog                |   1 +
>  libavformat/Makefile     |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/pp_bnk.c     | 218 +++++++++++++++++++++++++++++++++++++++
>  libavformat/version.h    |   2 +-
>  5 files changed, 222 insertions(+), 1 deletion(-)
>  create mode 100644 libavformat/pp_bnk.c
> 
> diff --git a/Changelog b/Changelog
> index 927dc89c51..0970bfb11e 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -55,6 +55,7 @@ version <next>:
>  - CRI HCA decoder
>  - CRI HCA demuxer
>  - Cunning Developments ADPCM decoder
> +- Pro Pinball Series Soundbank demuxer
>  
>  
>  version 4.2:
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 8fd0d43721..9df99133fa 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -428,6 +428,7 @@ OBJS-$(CONFIG_PCM_VIDC_DEMUXER)          += pcmdec.o pcm.o
>  OBJS-$(CONFIG_PCM_VIDC_MUXER)            += pcmenc.o rawenc.o
>  OBJS-$(CONFIG_PJS_DEMUXER)               += pjsdec.o subtitles.o
>  OBJS-$(CONFIG_PMP_DEMUXER)               += pmpdec.o
> +OBJS-$(CONFIG_PP_BNK_DEMUXER)            += pp_bnk.o
>  OBJS-$(CONFIG_PVA_DEMUXER)               += pva.o
>  OBJS-$(CONFIG_PVF_DEMUXER)               += pvfdec.o pcm.o
>  OBJS-$(CONFIG_QCP_DEMUXER)               += qcp.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 39d2c352f5..3919c9e4c1 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -341,6 +341,7 @@ extern AVInputFormat  ff_pcm_u8_demuxer;
>  extern AVOutputFormat ff_pcm_u8_muxer;
>  extern AVInputFormat  ff_pjs_demuxer;
>  extern AVInputFormat  ff_pmp_demuxer;
> +extern AVInputFormat  ff_pp_bnk_demuxer;
>  extern AVOutputFormat ff_psp_muxer;
>  extern AVInputFormat  ff_pva_demuxer;
>  extern AVInputFormat  ff_pvf_demuxer;
> diff --git a/libavformat/pp_bnk.c b/libavformat/pp_bnk.c
> new file mode 100644
> index 0000000000..dbb160576e
> --- /dev/null
> +++ b/libavformat/pp_bnk.c
> @@ -0,0 +1,218 @@
> +/*
> + * Pro Pinball Series Soundbank (44c, 22c, 11c, 5c) demuxer.
> + *
> + * Copyright (C) 2020 Zane van Iperen (zane@zanevaniperen.com)
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#include "avformat.h"
> +#include "internal.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/avassert.h"
> +#include "libavutil/internal.h"
> +
> +#define PP_BNK_MAX_READ_SIZE    4096
> +#define PP_BNK_FILE_HEADER_SIZE 20
> +#define PP_BNK_TRACK_SIZE       20
> +
> +typedef struct PPBnkHeader {
> +    uint32_t        bank_id;        /*< Bank ID, useless for our purposes. */
> +    uint32_t        sample_rate;    /*< Sample rate of the contained tracks. */
> +    uint32_t        always1;        /*< Unknown, always seems to be 1. */
> +    uint32_t        track_count;    /*< Number of tracks in the file. */
> +    uint32_t        unknown;        /*< Possibly flags. 2 == music, 0 == sfx? */
> +} PPBnkHeader;
> +
> +typedef struct PPBnkTrack {
> +    uint32_t        id;             /*< Track ID. Usually track[i].id == track[i-1].id + 1, but not always */
> +    uint32_t        size;           /*< Size of the data in bytes. */
> +    uint32_t        sample_rate;    /*< Sample rate. */
> +    uint32_t        always1_1;      /*< Unknown, always seems to be 1. */
> +    uint32_t        always1_2;      /*< Unknown, always seems to be 1. */
> +} PPBnkTrack;
> +
> +typedef struct PPBnkCtxTrack {
> +    int64_t         data_offset;
> +    uint32_t        data_size;
> +} PPBnkCtxTrack;
> +
> +typedef struct PPBnkCtx {
> +    int             track_count;
> +    PPBnkCtxTrack   *tracks;
> +    uint32_t        i;

How about using a more descriptive name for this like current_track?

> +    uint32_t        bytes_read;
> +} PPBnkCtx;
> +
> +static void pp_bnk_parse_header(PPBnkHeader *hdr, const uint8_t *buf)
> +{
> +    hdr->bank_id        = AV_RL32(buf +  0);
> +    hdr->sample_rate    = AV_RL32(buf +  4);
> +    hdr->always1        = AV_RL32(buf +  8);
> +    hdr->track_count    = AV_RL32(buf + 12);
> +    hdr->unknown        = AV_RL32(buf + 16);
> +}
> +
> +static void pp_bnk_parse_track(PPBnkTrack *trk, const uint8_t *buf)
> +{
> +    trk->id             = AV_RL32(buf +  0);
> +    trk->size           = AV_RL32(buf +  4);
> +    trk->sample_rate    = AV_RL32(buf +  8);
> +    trk->always1_1      = AV_RL32(buf + 12);
> +    trk->always1_2      = AV_RL32(buf + 16);
> +}
> +
> +static int pp_bnk_read_header(AVFormatContext *s)
> +{
> +    int ret;
> +    AVStream *st;
> +    AVCodecParameters *par;
> +    PPBnkCtx *ctx = s->priv_data;
> +    uint8_t buf[FFMAX(PP_BNK_FILE_HEADER_SIZE, PP_BNK_TRACK_SIZE)];
> +    PPBnkHeader hdr;
> +
> +    if ((ret = avio_read(s->pb, buf, PP_BNK_FILE_HEADER_SIZE)) < 0)
> +        return ret;
> +    else if (ret != PP_BNK_FILE_HEADER_SIZE)
> +        return AVERROR(EIO);
> +
> +    pp_bnk_parse_header(&hdr, buf);
> +
> +    if (hdr.track_count == 0 || hdr.track_count > INT_MAX)
> +        return AVERROR_INVALIDDATA;
> +
> +    if (hdr.sample_rate == 0 || hdr.sample_rate > INT_MAX)
> +        return AVERROR_INVALIDDATA;
> +
> +    if (hdr.always1 != 1) {
> +        avpriv_request_sample(s, "Non-one header value");
> +        return AVERROR_PATCHWELCOME;
> +    }
> +
> +    ctx->track_count = hdr.track_count;
> +    if ((ret = av_reallocp_array(&ctx->tracks, hdr.track_count, sizeof(PPBnkCtxTrack))))

read_close is not called if read_header fails and therefore this array
leaks if any of the following things that can fail fail.
(It would probably make sense to call it automatically, but this will
require changes in some demuxers.)

> +        return ret;
> +
> +    /* 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)
> +            return ret;
> +        else if (ret != PP_BNK_TRACK_SIZE)
> +            return AVERROR(EIO);
> +
> +        pp_bnk_parse_track(&e, buf);
> +
> +        /* The individual sample rates of all tracks must match that of the file header. */
> +        if (e.sample_rate != hdr.sample_rate) {
> +            return AVERROR_INVALIDDATA;
> +        }
> +
> +        ctx->tracks[i].data_offset = avio_tell(s->pb);
> +        ctx->tracks[i].data_size   = e.size;
> +
> +        /* Skip over the data to the next stream header. */
> +        avio_skip(s->pb, e.size);
> +    }
> +
> +    /* Build the streams. */
> +    for (int i = 0; i < hdr.track_count; i++) {
> +
> +        if (!(st = avformat_new_stream(s, NULL)))
> +            return AVERROR(ENOMEM);
> +
> +        par                         = st->codecpar;
> +        par->codec_type             = AVMEDIA_TYPE_AUDIO;
> +        par->codec_id               = AV_CODEC_ID_ADPCM_IMA_CUNNING;
> +        par->format                 = AV_SAMPLE_FMT_S16;
> +        par->channel_layout         = AV_CH_LAYOUT_MONO;
> +        par->channels               = 1;
> +        par->sample_rate            = hdr.sample_rate;
> +        par->bits_per_coded_sample  = 4;
> +        par->bits_per_raw_sample    = 16;
> +        par->block_align            = 1;
> +        par->bit_rate               = par->sample_rate * par->bits_per_coded_sample;
> +
> +        avpriv_set_pts_info(st, 64, 1, par->sample_rate);
> +        st->start_time              = 0;
> +        st->duration                = ctx->tracks[i].data_size * 2;
> +    }
> +
> +    /* Seek to the start of the first stream. */
> +    if ((ret = avio_seek(s->pb, ctx->tracks[0].data_offset, SEEK_SET)) < 0)
> +        return ret;
> +    else if (ret != ctx->tracks[0].data_offset)
> +        return AVERROR(EIO);
> +
> +    return 0;
> +}
> +
> +static int pp_bnk_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    int ret, size;
> +    PPBnkCtx *ctx = s->priv_data;
> +    PPBnkCtxTrack *trk = ctx->tracks + ctx->i;
> +
> +    av_assert0(ctx->bytes_read <= trk->data_size);
> +
> +    if (ctx->bytes_read == trk->data_size) {
> +        ctx->bytes_read  = 0;
> +        ctx->i          += 1;
> +        trk             += 1;

Why don't you simply use an increment ++?

> +
> +        if ((ret = avio_seek(s->pb, trk->data_offset, SEEK_SET)) < 0)

avio_seek returns int64_t, yet ret is only an int. This will cause
problems when you seek to positions from 2GB to 4GB.

> +            return ret;
> +        else if (ret != trk->data_offset)
> +            return AVERROR(EIO);

I wonder whether the seek should be attempted before you set bytes_read
and i. If the seek fails and the caller retries to read a packet, it
will (try to) read the wrong data.

> +    }
> +
> +    if (ctx->i == ctx->track_count)
> +        return AVERROR_EOF;
> +
> +    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;
> +
> +    pkt->stream_index   = ctx->i;
> +    pkt->duration       = ret * 2;
> +    return 0;
> +}
> +
> +static int pp_bnk_read_close(AVFormatContext *s)
> +{
> +    PPBnkCtx *ctx = s->priv_data;
> +
> +    if (ctx->tracks)
> +        av_freep(&ctx->tracks);

The check here is unnecessary as av_freep() already checks for this; and
moreover, read_close is only called if read_header succeeded, so
ctx->tracks is always != NULL.

> +
> +    return 0;
> +}
> +
> +AVInputFormat ff_pp_bnk_demuxer = {
> +    .name           = "pp_bnk",
> +    .long_name      = NULL_IF_CONFIG_SMALL("Pro Pinball Series Soundbank"),
> +    .priv_data_size = sizeof(PPBnkCtx),
> +    .read_header    = pp_bnk_read_header,
> +    .read_packet    = pp_bnk_read_packet,
> +    .read_close     = pp_bnk_read_close,
> +    .extensions     = "5c,11c,22c,44c"
> +};
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 18c2f5fec2..493a0b337f 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -32,7 +32,7 @@
>  // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
>  // Also please add any ticket numbers that you believe might be affected here
>  #define LIBAVFORMAT_VERSION_MAJOR  58
> -#define LIBAVFORMAT_VERSION_MINOR  42
> +#define LIBAVFORMAT_VERSION_MINOR  43
>  #define LIBAVFORMAT_VERSION_MICRO 100
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
>

On Wed, 18 Mar 2020 16:05:40 +0100
"Andreas Rheinhardt" <andreas.rheinhardt@gmail.com> wrote:

> > +
> > +typedef struct PPBnkCtx {
> > +    int             track_count;
> > +    PPBnkCtxTrack   *tracks;
> > +    uint32_t        i;  
> 
> How about using a more descriptive name for this like current_track?
> 

Done.


> > +
> > +    ctx->track_count = hdr.track_count;
> > +    if ((ret = av_reallocp_array(&ctx->tracks, hdr.track_count,
> > sizeof(PPBnkCtxTrack))))  
> 
> read_close is not called if read_header fails and therefore this array
> leaks if any of the following things that can fail fail.
> (It would probably make sense to call it automatically, but this will
> require changes in some demuxers.)
> 

Fixed, I assumed it was called irregardless of failure.

I wonder how much work it would be to port all of the demuxers to
handle this...


> > +        ctx->bytes_read  = 0;
> > +        ctx->i          += 1;
> > +        trk             += 1;  
> 
> Why don't you simply use an increment ++?
> 

Stylistic reasons. Is moot anyway as I had to change that code to fix a
different bug.

> > +
> > +        if ((ret = avio_seek(s->pb, trk->data_offset, SEEK_SET)) <
> > 0)  
> 
> avio_seek returns int64_t, yet ret is only an int. This will cause
> problems when you seek to positions from 2GB to 4GB.
> 

Fixed.

> > +            return ret;
> > +        else if (ret != trk->data_offset)
> > +            return AVERROR(EIO);  
> 
> I wonder whether the seek should be attempted before you set
> bytes_read and i. If the seek fails and the caller retries to read a
> packet, it will (try to) read the wrong data.
> 

Fixed. This lead me to another issue where it overreads trk before
checking for EOF (also fixed).


> > +static int pp_bnk_read_close(AVFormatContext *s)
> > +{
> > +    PPBnkCtx *ctx = s->priv_data;
> > +
> > +    if (ctx->tracks)
> > +        av_freep(&ctx->tracks);  
> 
> The check here is unnecessary as av_freep() already checks for this;
> and moreover, read_close is only called if read_header succeeded, so
> ctx->tracks is always != NULL.

Fixed.

Zane

On Wed, 18 Mar 2020 15:19:19 +0100
"Carl Eugen Hoyos" <ceffmpeg@gmail.com> wrote:
> 
> Any restrictions on track_count and sample_rate that can be used for
> auto-detection?
> 

I've done a preliminary probe function. How's this?

On all the files I tested, I received one of two values:
99 - For music (these have an extra condition I can test for)
87 - For everything else

static int pp_bnk_probe(const AVProbeData *p)
{
    PPBnkHeader hdr;
    uint32_t expected_sample_rate;
    int score;

    pp_bnk_parse_header(&hdr, p->buf);

    if (hdr.track_count == 0 || hdr.sample_rate == 0)
        return 0;

    score = 0;

    /* These limits are based on analysing the game files. */
    if (hdr.track_count <= 113)
        score += AVPROBE_SCORE_MAX / 4;

    if (hdr.sample_rate <= 44100)
        score += AVPROBE_SCORE_MAX / 4;

    if ((hdr.flags & ~PP_BNK_FLAG_MASK) == 0)
        score += AVPROBE_SCORE_MAX / 4;

    if ((hdr.flags & PP_BNK_FLAG_MUSIC) && hdr.track_count == 2)
        score += AVPROBE_SCORE_MAX / 8;

    /* Also check based on file extension, if we have it. */
    if (av_match_ext(p->filename, "5c"))
        expected_sample_rate = 5512;
    else if (av_match_ext(p->filename, "11c"))
        expected_sample_rate = 11025;
    else if (av_match_ext(p->filename, "22c"))
        expected_sample_rate = 22050;
    else if (av_match_ext(p->filename, "44c"))
        expected_sample_rate = 44100;
    else
        expected_sample_rate = 0;

    if (expected_sample_rate && hdr.sample_rate == expected_sample_rate)
        score += AVPROBE_SCORE_MAX / 8;

    return score;
}

Zane

Am Fr., 27. März 2020 um 15:25 Uhr schrieb Zane van Iperen
<zane@zanevaniperen.com>:
>
> On Wed, 18 Mar 2020 15:19:19 +0100
> "Carl Eugen Hoyos" <ceffmpeg@gmail.com> wrote:
> >
> > Any restrictions on track_count and sample_rate that can be used for
> > auto-detection?
> >
>
> I've done a preliminary probe function. How's this?
>
> On all the files I tested, I received one of two values:
> 99 - For music (these have an extra condition I can test for)
> 87 - For everything else
>
> static int pp_bnk_probe(const AVProbeData *p)
> {
>     PPBnkHeader hdr;
>     uint32_t expected_sample_rate;
>     int score;
>
>     pp_bnk_parse_header(&hdr, p->buf);
>
>     if (hdr.track_count == 0 || hdr.sample_rate == 0)
>         return 0;
>
>     score = 0;
>
>     /* These limits are based on analysing the game files. */
>     if (hdr.track_count <= 113)
>         score += AVPROBE_SCORE_MAX / 4;
>
>     if (hdr.sample_rate <= 44100)
>         score += AVPROBE_SCORE_MAX / 4;
>
>     if ((hdr.flags & ~PP_BNK_FLAG_MASK) == 0)
>         score += AVPROBE_SCORE_MAX / 4;
>
>     if ((hdr.flags & PP_BNK_FLAG_MUSIC) && hdr.track_count == 2)
>         score += AVPROBE_SCORE_MAX / 8;
>
>     /* Also check based on file extension, if we have it. */
>     if (av_match_ext(p->filename, "5c"))
>         expected_sample_rate = 5512;
>     else if (av_match_ext(p->filename, "11c"))
>         expected_sample_rate = 11025;
>     else if (av_match_ext(p->filename, "22c"))
>         expected_sample_rate = 22050;
>     else if (av_match_ext(p->filename, "44c"))
>         expected_sample_rate = 44100;
>     else
>         expected_sample_rate = 0;
>
>     if (expected_sample_rate && hdr.sample_rate == expected_sample_rate)
>         score += AVPROBE_SCORE_MAX / 8;
>
>     return score;

Apart from what I wrote on irc:
What surprises me most is that iirc, you explained that the sample_rate
is written repeatedly to the file and is always identical in one file. Why
is that not checked?

You seem to have a fine (and small) collection of sample rates to check against.

Carl Eugen

On Sun, 29 Mar 2020 01:49:32 +0100
"Carl Eugen Hoyos" <ceffmpeg@gmail.com> wrote:


> Apart from what I wrote on irc:
> What surprises me most is that iirc, you explained that the
> sample_rate is written repeatedly to the file and is always identical
> in one file. Why is that not checked?
> 

The main reason is that the track headers are spread out all over the
file, which will almost certainly be larger than the probe buffer.

The best I can do is check that the sample rate of the first track
matches that of the file (as it's immediately after the main header).
I do this in the next version of the patch, which I will be posting
later.

> You seem to have a fine (and small) collection of sample rates to
> check against.
> 
> Carl Eugen

Zane