[FFmpeg-devel] [PATCH] avformat/amr: reduce raw amr false positive detection

[Carl Eugen Hoyos]

Am Mi., 2. Okt. 2019 um 22:18 Uhr schrieb Carl Eugen Hoyos <ceffmpeg at gmail.com>:
>
> Am Mi., 2. Okt. 2019 um 20:19 Uhr schrieb Paul B Mahol <onemda at gmail.com>:
> >
> > Signed-off-by: Paul B Mahol <onemda at gmail.com>
> > ---
> >  libavformat/amr.c | 18 ++++++++++++------
> >  1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavformat/amr.c b/libavformat/amr.c
> > index 42840a50a3..600cb1b0f0 100644
> > --- a/libavformat/amr.c
> > +++ b/libavformat/amr.c
> > @@ -190,9 +190,12 @@ static int amrnb_probe(const AVProbeData *p)
> >                  if (b[++i] != last)
> >                      break;
> >              }
> > -            if (size > 0) {
> > -                valid++;
> > -                i += size;
> > +            while (size--) {
> > +                if (b[i])
> > +                    valid++;
> > +                else
> > +                    invalid++;
> > +                i++;
> >              }
> >          } else {
> >              valid = 0;
> > @@ -246,9 +249,12 @@ static int amrwb_probe(const AVProbeData *p)
> >                  if (b[++i] != last)
> >                      break;
> >              }
> > -            if (size > 0) {
> > -                valid++;
> > -                i += size;
> > +            while (size--) {
> > +                if (b[i])
> > +                    valid++;
> > +                else
> > +                    invalid++;
> > +                i++;
> >              }
> >          } else {
> >              valid = 0;
>
> The changes to amrwb are unneeded for the file you provided.
> (A "PGP Secret Sub-key" according to file.)
>
> I encoded a few amr files and "00 00" seems common and not
> invalid.
>
> The following works here but I believe the main issue is that
> our amr decoder happily decodes the "PGP Secret Sub-key"
> without any error messages so I wonder if the detection is
> wrong at all.
>
> diff --git a/libavformat/amr.c b/libavformat/amr.c
> index 42840a50a3..2645af95c2 100644
> --- a/libavformat/amr.c
> +++ b/libavformat/amr.c
> @@ -186,8 +186,18 @@ static int amrnb_probe(const AVProbeData *p)
>          if (mode < 9 && (b[i] & 0x4) == 0x4) {
>              int last = b[i];
>              int size = amrnb_packed_size[mode];
> +            int changes = 0, repeats = 0;
>              while (size--) {
>                  if (b[++i] != last)
> +                    changes++;
> +                else
> +                    repeats++;
> +                last = b[i];
> +                if (repeats >= 2) {
> +                    i += size;
> +                    size = 0;
> +                    break;
> +                } else if (changes > 4)
>                      break;
>              }
>              if (size > 0) {
> @@ -200,7 +210,7 @@ static int amrnb_probe(const AVProbeData *p)
>              i++;
>          }
>      }
> -    if (valid > 100 && valid >> 4 > invalid)
> +    if (valid > 120 && valid >> 4 > invalid)

Another idea is to require different modes, that should be simpler if you
believe that the sample should not get detected.

Carl Eugen