mp3_skiptags: let it consume all the tags at file end (bug #4907):
authorOzkan Sezer
Fri, 20 Dec 2019 11:50:05 +0300
changeset 11034e1c2282e6f1
parent 1102 91dc8a87aaee
child 1105 fabc6a1bc76c
mp3_skiptags: let it consume all the tags at file end (bug #4907):

We do not know the order of ape, or lyrics3, or musicmatch tags,
so we loop until we consume all, scanning for each tag type once.
I don't yet care about freaky broken mp3 files with double tags.

<rant> MP3 standard has no metadata format, so everyone invented
their own thing, even with extensions, until ID3v2 became dominant:
Hence the impossible mess there.</rant>

Also remove inline directive from a few detection procedures there.
src/codecs/mp3utils.c
     1.1 --- a/src/codecs/mp3utils.c	Fri Dec 20 11:50:05 2019 +0300
     1.2 +++ b/src/codecs/mp3utils.c	Fri Dec 20 11:50:05 2019 +0300
     1.3 @@ -68,7 +68,7 @@
     1.4      }
     1.5      return SDL_TRUE;
     1.6  }
     1.7 -static SDL_INLINE SDL_bool is_id3v2(const unsigned char *data, size_t length)
     1.8 +static SDL_bool is_id3v2(const unsigned char *data, size_t length)
     1.9  {
    1.10      /* ID3v2 header is 10 bytes:  http://id3.org/id3v2.4.0-structure */
    1.11      /* bytes 0-2: "ID3" identifier */
    1.12 @@ -87,7 +87,7 @@
    1.13      }
    1.14      return SDL_TRUE;
    1.15  }
    1.16 -static SDL_INLINE long get_id3v2_len(const unsigned char *data, long length)
    1.17 +static long get_id3v2_len(const unsigned char *data, long length)
    1.18  {
    1.19      /* size is a 'synchsafe' integer (see above) */
    1.20      long size = (long)((data[6]<<21) + (data[7]<<14) + (data[8]<<7) + data[9]);
    1.21 @@ -104,7 +104,7 @@
    1.22      }
    1.23      return size;
    1.24  }
    1.25 -static SDL_INLINE SDL_bool is_apetag(const unsigned char *data, size_t length)
    1.26 +static SDL_bool is_apetag(const unsigned char *data, size_t length)
    1.27  {
    1.28     /* http://wiki.hydrogenaud.io/index.php?title=APEv2_specification
    1.29      * Header/footer is 32 bytes: bytes 0-7 ident, bytes 8-11 version,
    1.30 @@ -124,7 +124,7 @@
    1.31      }
    1.32      return SDL_TRUE;
    1.33  }
    1.34 -static SDL_INLINE long get_ape_len(const unsigned char *data)
    1.35 +static long get_ape_len(const unsigned char *data)
    1.36  {
    1.37      Uint32 flags, version;
    1.38      long size = (long)((data[15]<<24) | (data[14]<<16) | (data[13]<<8) | data[12]);
    1.39 @@ -141,7 +141,7 @@
    1.40      if (SDL_memcmp(data+6,"LYRICSEND",9) == 0) return 1; /* v1 */
    1.41      return 0;
    1.42  }
    1.43 -static SDL_INLINE long get_lyrics3v1_len(struct mp3file_t *m) {
    1.44 +static long get_lyrics3v1_len(struct mp3file_t *m) {
    1.45      const char *p; long i, len;
    1.46      char buf[5104];
    1.47      /* needs manual search:  http://id3.org/Lyrics3 */
    1.48 @@ -163,13 +163,13 @@
    1.49      if (length != 6) return 0;
    1.50      return SDL_strtol((const char *)data, NULL, 10) + 15;
    1.51  }
    1.52 -static SDL_INLINE SDL_bool verify_lyrics3v2(const unsigned char *data, long length) {
    1.53 +static SDL_bool verify_lyrics3v2(const unsigned char *data, long length) {
    1.54      if (length < 11) return SDL_FALSE;
    1.55      if (SDL_memcmp(data,"LYRICSBEGIN",11) == 0) return SDL_TRUE;
    1.56      return SDL_FALSE;
    1.57  }
    1.58  #define MMTAG_PARANOID
    1.59 -static SDL_INLINE SDL_bool is_musicmatch(const unsigned char *data, long length) {
    1.60 +static SDL_bool is_musicmatch(const unsigned char *data, long length) {
    1.61    /* From docs/musicmatch.txt in id3lib: https://sourceforge.net/projects/id3lib/
    1.62       Overall tag structure:
    1.63  
    1.64 @@ -212,7 +212,7 @@
    1.65      #endif
    1.66      return SDL_TRUE;
    1.67  }
    1.68 -static SDL_INLINE long get_musicmatch_len(struct mp3file_t *m) {
    1.69 +static long get_musicmatch_len(struct mp3file_t *m) {
    1.70      const Sint32 metasizes[4] = { 7868, 7936, 8004, 8132 };
    1.71      const unsigned char syncstr[10] = {'1','8','2','7','3','6','4','5',0,0};
    1.72      unsigned char buf[256];
    1.73 @@ -279,12 +279,90 @@
    1.74      return len + 256; /* header is present. */
    1.75  }
    1.76  
    1.77 +static int probe_id3v1(struct mp3file_t *fil, unsigned char *buf) {
    1.78 +    if (fil->length >= 128) {
    1.79 +        MP3_RWseek(fil, -128, RW_SEEK_END);
    1.80 +        if (MP3_RWread(fil, buf, 1, 128) != 128)
    1.81 +            return -1;
    1.82 +        if (is_id3v1(buf, 128)) {
    1.83 +            fil->length -= 128;
    1.84 +            return 1;
    1.85 +            /* FIXME: handle possible double-ID3v1 tags?? */
    1.86 +        }
    1.87 +    }
    1.88 +    return 0;
    1.89 +}
    1.90 +static int probe_mmtag(struct mp3file_t *fil, unsigned char *buf) {
    1.91 +    long len;
    1.92 +    if (fil->length >= 68) {
    1.93 +        MP3_RWseek(fil, -48, RW_SEEK_END);
    1.94 +        if (MP3_RWread(fil, buf, 1, 48) != 48)
    1.95 +            return -1;
    1.96 +        if (is_musicmatch(buf, 48)) {
    1.97 +            len = get_musicmatch_len(fil);
    1.98 +            if (len < 0) return -1;
    1.99 +            if (len >= fil->length) return -1;
   1.100 +            fil->length -= len;
   1.101 +            return 1;
   1.102 +        }
   1.103 +    }
   1.104 +    return 0;
   1.105 +}
   1.106 +static int probe_apetag(struct mp3file_t *fil, unsigned char *buf) {
   1.107 +    long len;
   1.108 +    if (fil->length >= 32) {
   1.109 +        MP3_RWseek(fil, -32, RW_SEEK_END);
   1.110 +        if (MP3_RWread(fil, buf, 1, 32) != 32)
   1.111 +            return -1;
   1.112 +        if (is_apetag(buf, 32)) {
   1.113 +            len = get_ape_len(buf);
   1.114 +            if (len >= fil->length) return -1;
   1.115 +            fil->length -= len;
   1.116 +            return 1;
   1.117 +        }
   1.118 +    }
   1.119 +    return 0;
   1.120 +}
   1.121 +static int probe_lyrics3(struct mp3file_t *fil, unsigned char *buf) {
   1.122 +    long len;
   1.123 +    if (fil->length >= 15) {
   1.124 +        MP3_RWseek(fil, -15, RW_SEEK_END);
   1.125 +        if (MP3_RWread(fil, buf, 1, 15) != 15)
   1.126 +            return -1;
   1.127 +        len = is_lyrics3tag(buf, 15);
   1.128 +        if (len == 2) {
   1.129 +            len = get_lyrics3v2_len(buf, 6);
   1.130 +            if (len >= fil->length) return -1;
   1.131 +            if (len < 15) return -1;
   1.132 +            MP3_RWseek(fil, -len, RW_SEEK_END);
   1.133 +            if (MP3_RWread(fil, buf, 1, 11)!= 11)
   1.134 +                return -1;
   1.135 +            if (!verify_lyrics3v2(buf, 11)) return -1;
   1.136 +            fil->length -= len;
   1.137 +            return 1;
   1.138 +        }
   1.139 +        else if (len == 1) {
   1.140 +            len = get_lyrics3v1_len(fil);
   1.141 +            if (len < 0) return -1;
   1.142 +            fil->length -= len;
   1.143 +            return 1;
   1.144 +        }
   1.145 +    }
   1.146 +    return 0;
   1.147 +}
   1.148 +
   1.149  int mp3_skiptags(struct mp3file_t *fil, SDL_bool keep_id3v2)
   1.150  {
   1.151      unsigned char buf[128];
   1.152      long len; size_t readsize;
   1.153 +    int c_id3, c_ape, c_lyr, c_mm;
   1.154      int rc = -1;
   1.155  
   1.156 +    /* MP3 standard has no metadata format, so everyone invented
   1.157 +     * their own thing, even with extensions, until ID3v2 became
   1.158 +     * dominant: Hence the impossible mess here.
   1.159 +     */
   1.160 +
   1.161      readsize = MP3_RWread(fil, buf, 1, 128);
   1.162      if (!readsize) goto fail;
   1.163  
   1.164 @@ -292,7 +370,7 @@
   1.165      if (is_id3v2(buf, readsize)) {
   1.166          len = get_id3v2_len(buf, (long)readsize);
   1.167          if (len >= fil->length) goto fail;
   1.168 -        if (! keep_id3v2)  {
   1.169 +        if (! keep_id3v2) {
   1.170              fil->start  += len;
   1.171              fil->length -= len;
   1.172          }
   1.173 @@ -306,66 +384,38 @@
   1.174          fil->length -= len;
   1.175      }
   1.176  
   1.177 -    /* ID3v1 tag is at the end */
   1.178 -    if (fil->length >= 128) {
   1.179 -        MP3_RWseek(fil, -128, RW_SEEK_END);
   1.180 -        readsize = MP3_RWread(fil, buf, 1, 128);
   1.181 -        if (readsize != 128) goto fail;
   1.182 -        if (is_id3v1(buf, 128)) {
   1.183 -            fil->length -= 128;
   1.184 -            /* FIXME: handle possible double-ID3v1 tags?? */
   1.185 -        }
   1.186 +    /* it's not impossible that _old_ MusicMatch tag
   1.187 +     * placing itself after ID3v1. */
   1.188 +    if ((c_mm = probe_mmtag(fil, buf)) < 0) {
   1.189 +        goto fail;
   1.190      }
   1.191 -
   1.192 -    /* do we know whether ape or lyrics3 is the first?
   1.193 -     * well, we don't: we need to handle that later... */
   1.194 -
   1.195 -    /* check for the _old_ MusicMatch tag at end. */
   1.196 -    if (fil->length >= 68) {
   1.197 -        MP3_RWseek(fil, -48, RW_SEEK_END);
   1.198 -        readsize = MP3_RWread(fil, buf, 1, 48);
   1.199 -        if (readsize != 48) goto fail;
   1.200 -        if (is_musicmatch(buf, 48)) {
   1.201 -            len = get_musicmatch_len(fil);
   1.202 -            if (len < 0) goto fail;
   1.203 -            if (len >= fil->length) goto fail;
   1.204 -            fil->length -= len;
   1.205 -        }
   1.206 +    /* ID3v1 tag is at the end */
   1.207 +    if ((c_id3 = probe_id3v1(fil, buf)) < 0) {
   1.208 +        goto fail;
   1.209      }
   1.210 -
   1.211 -    /* APE tag may be at the end: read the footer */
   1.212 -    if (fil->length >= 32) {
   1.213 -        MP3_RWseek(fil, -32, RW_SEEK_END);
   1.214 -        readsize = MP3_RWread(fil, buf, 1, 32);
   1.215 -        if (readsize != 32) goto fail;
   1.216 -        if (is_apetag(buf, 32)) {
   1.217 -            len = get_ape_len(buf);
   1.218 -            if (len >= fil->length) goto fail;
   1.219 -            fil->length -= len;
   1.220 +    /* we do not know the order of ape or lyrics3
   1.221 +     * or musicmatch tags, hence the loop here.. */
   1.222 +    c_ape = 0;
   1.223 +    c_lyr = 0;
   1.224 +    for (;;) {
   1.225 +        if (!c_lyr) {
   1.226 +        /* care about mp3s with double Lyrics3 tags? */
   1.227 +            if ((c_lyr = probe_lyrics3(fil, buf)) < 0)
   1.228 +                goto fail;
   1.229 +            if (c_lyr) continue;
   1.230          }
   1.231 -    }
   1.232 -
   1.233 -    if (fil->length >= 15) {
   1.234 -        MP3_RWseek(fil, -15, RW_SEEK_END);
   1.235 -        readsize = MP3_RWread(fil, buf, 1, 15);
   1.236 -        if (readsize != 15) goto fail;
   1.237 -        len = is_lyrics3tag(buf, 15);
   1.238 -        if (len == 2) {
   1.239 -            len = get_lyrics3v2_len(buf, 6);
   1.240 -            if (len >= fil->length) goto fail;
   1.241 -            if (len < 15) goto fail;
   1.242 -            MP3_RWseek(fil, -len, RW_SEEK_END);
   1.243 -            readsize = MP3_RWread(fil, buf, 1, 11);
   1.244 -            if (readsize != 11) goto fail;
   1.245 -            if (!verify_lyrics3v2(buf, 11)) goto fail;
   1.246 -            fil->length -= len;
   1.247 +        if (!c_mm) {
   1.248 +            if ((c_mm = probe_mmtag(fil, buf)) < 0)
   1.249 +                goto fail;
   1.250 +            if (c_mm) continue;
   1.251          }
   1.252 -        else if (len == 1) {
   1.253 -            len = get_lyrics3v1_len(fil);
   1.254 -            if (len < 0) goto fail;
   1.255 -            fil->length -= len;
   1.256 +        if (!c_ape) {
   1.257 +            if ((c_ape = probe_apetag(fil, buf)) < 0)
   1.258 +                goto fail;
   1.259 +            if (c_ape) continue;
   1.260          }
   1.261 -    }
   1.262 +        break;
   1.263 +    } /* for (;;) */
   1.264  
   1.265      rc = (fil->length > 0)? 0 : -1;
   1.266      fail: