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

[Paul B Mahol]

On 1/27/20, Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
> 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.

No it can not, check your math skills again.

I expected you would give more constructive review.

>
>> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".