[FFmpeg-devel] [PATCH] avformat/tty: add probe function

[Carl Eugen Hoyos]

Am Mo., 27. Jan. 2020 um 22:44 Uhr schrieb Paul B Mahol <onemda at gmail.com>:
>
> On 1/27/20, Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
> > Am Mo., 27. Jan. 2020 um 22:01 Uhr schrieb Paul B Mahol <onemda at gmail.com>:
> >>
> >> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> >> ---
> >>  libavformat/tty.c | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/libavformat/tty.c b/libavformat/tty.c
> >> index 8d48f2c45c..acc6da27cc 100644
> >> --- a/libavformat/tty.c
> >> +++ b/libavformat/tty.c
> >> @@ -24,6 +24,8 @@
> >>   * Tele-typewriter demuxer
> >>   */
> >>
> >> +#include <ctype.h>
> >> +
> >>  #include "libavutil/intreadwrite.h"
> >>  #include "libavutil/avstring.h"
> >>  #include "libavutil/log.h"
> >> @@ -42,6 +44,16 @@ typedef struct TtyDemuxContext {
> >>      AVRational framerate; /**< Set by a private option. */
> >>  } TtyDemuxContext;
> >>
> >> +static int read_probe(const AVProbeData *p)
> >> +{
> >> +    int64_t cnt = 0;
> >> +
> >> +    for (int i = 0; i < p->buf_size; i++)
> >> +        cnt += !!isalnum(p->buf[i]);
> >> +
> >> +    return cnt * 100 / p->buf_size;
> >
> > The function looks as if it could return scores
> > higher than SCORE_MAX and as if it would
> > return high scores for very small probe size.
> > Both are not good, normally we test for both
> > a minimum number of counts and a high
> > percentage of correct values.
>
> Why are you propagandizing untrue statements here?

Well, it is true that the function looked to me as if
it could return scores > 100 (which it does not).

> Score returned by this probe can not be bigger than 100 and less than 0.

It can return 100 for very small probe size which is not good.

> Probe function works fine and adding extension is pointless.

You could only return a high score if your probe heuristic
agrees and if the extension matches.

> >> +}
> >> +
> >>  /**
> >>   * Parse EFI header
> >>   */
> >> @@ -153,6 +165,7 @@ AVInputFormat ff_tty_demuxer = {
> >>      .name           = "tty",
> >>      .long_name      = NULL_IF_CONFIG_SMALL("Tele-typewriter"),
> >>      .priv_data_size = sizeof(TtyDemuxContext),
> >> +    .read_probe     = read_probe,
> >>      .read_header    = read_header,
> >>      .read_packet    = read_packet,
> >>      .extensions     = "ans,art,asc,diz,ice,nfo,txt,vt",
> >
> > You could add the extensions to above test.
> >
> > Background: An mpv user uploaded a jpg sample to the mpv
> > bug tracker and renamed it to sample1.txt. Because of its high
> > resolution and because the jpg probe function tries to distinguish
> > from mjpeg (which only works by checking if EOI is at EOF) the
> > extension returns a higher score than the jpg probe (the probe
> > function works fine if I remove the filename suffix),
> >
> > So another fix would be to merge the mjpeg demuxer with the
> > jpeg_pipe demuxer, I don't remember atm why I didn't do that.

Carl Eugen