[FFmpeg-devel] [PATCH 2/3] Add muxer/demuxer for raw codec2 and .c2 files

[Carl Eugen Hoyos]

2017-12-23 23:15 GMT+01:00 Tomas Härdin <tjoppen at acc.umu.se>:

> +//check for 0xC0DEC2, return non-zero if it doesn't match
> +static int check_magic(uint8_t *ptr) {

> +    return memcmp(ptr, avpriv_codec2_magic, 3);

AV_RB24(), or do I miss something?

> +static int codec2_probe(AVProbeData *p)
> +{
> +    int score;
> +
> +    //must be at least 7 bytes and start wih 0xC0DEC2

> +    if (p->buf_size < AVPRIV_CODEC2_HEADER_SIZE ||

This check is unneeded.

> check_magic(p->buf)) {
+        return 0;
+    }
+
+    //no .c2 files prior to 0.8

+    if (p->buf[3] == 0 && p->buf[4] < 8) {

You chose to define the versions, please use the defines here.

+        return 0;
+    }
+
+    //give a poor score if major version doesn't match
+    //this allows such files to be detected at least, even if we
can't do much with them
+    if (p->buf[3] != EXPECTED_CODEC2_MAJOR_VERSION) {
+        return AVPROBE_SCORE_MAX/10;
+    }

That may be ok.

+    //if the minor version is known, no unknown mode is used and no
flags are set then -> max score,
+    //else penalize 20% for each byte outside of expectations
+    //this way if only the first four bytes are OK then we're
slightly less than AVPROBE_SCORE_MAX/2
+    //cap score at max-1 unless file extension is .c2
+    score = AVPROBE_SCORE_MAX;

+    if (!av_match_ext(p->filename, "c2"))           score -= 1;

We don't do this for any other demuxer, if this makes sense, it should
be done in general code, not for a specific demuxer imo.

+    if (p->buf[4] >  EXPECTED_CODEC2_MINOR_VERSION) score -=
AVPROBE_SCORE_MAX/5;
+    if (p->buf[5] >  AVPRIV_CODEC2_MODE_MAX)        score -=
AVPROBE_SCORE_MAX/5;
+    if (p->buf[6] != 0)                             score -=
AVPROBE_SCORE_MAX/5;
+    return score;

Imo, this is too complicated:
If the first 32bit are correct, return MAX/2, for 24bit, return less

Returning MAX for less than 64bit seems wrong to me.

Thank you, Carl Eugen