len0rd/rockbox
1
0
Fork 1
mirror of https://github.com/Rockbox/rockbox.git synced 2025-12-07 05:35:02 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

FS#13497: '0' is sometimes a valid track number

Browse source 
Most notably for CD rips that use a track number of 0 for
the leadin.

Therefore change our "invalid track number" canary to -1 instead
of 0.  Additionally don't try to parse an empty string.

In the process, get rid of redudant 'discnum = 0' as well.

NOTE: While not strictly necessary, we recommend rebuilding the
      database to ensure files without track numbers are
      updated with the new canary.

Change-Id: I543f98ca49cec7b5eeffa7c14c1eca57171f345a
This commit is contained in:
Solomon Peachy 2025-11-19 21:03:34 -05:00
parent 481cc70fe0
commit e94a96cdcf
15 changed files with 104 additions and 89 deletions
Download patch file Download diff file Expand all files Collapse all files

2
apps/gui/skin_engine/skin_tokens.c
View file

@ -263,7 +263,7 @@ const char *get_id3_token(struct wps_token *token, struct mp3entry *id3,
case SKIN_TOKEN_METADATA_TRACK_NUMBER:
if (id3->track_string)
return id3->track_string;
if (id3->tracknum) {
if (id3->tracknum >= 0) {
itoa_buf(buf, buf_size, id3->tracknum);
return buf;
}

2
apps/plugins/lib/mul_id3.c
View file

@ -173,7 +173,7 @@ void finalize_id3(struct mp3entry *id3)
id3->codectype = mul_id3.codectype;
id3->vbr = mul_id3.vbr;
id3->discnum = 0;
id3->tracknum = 0;
id3->tracknum = -1;
id3->track_level = 0;
id3->album_level = 0;
}

2
apps/screens.c
View file

@ -571,7 +571,7 @@ static const char * id3_get_or_speak_info(int selected_item, void* data,
if(say_it)
say_number_and_spell(val, true);
}
else if (id3->tracknum)
else if (id3->tracknum >= 0)
{
itoa_buf(buffer, buffer_len, id3->tracknum);
val = buffer;

2
apps/tagcache.c
View file

@ -2322,7 +2322,7 @@ static void NO_INLINE add_tagcache(char *path, unsigned long mtime)
logf("-> %s", path);
if (id3.tracknum <= 0) /* Track number missing? */
if (id3.tracknum < 0) /* Track number missing? */
{
id3.tracknum = -1;
}

20
lib/rbcodec/metadata/a52.c
View file

@ -44,7 +44,7 @@ static const unsigned short a52_441framesizes[] =
};
bool get_a52_metadata(int fd, struct mp3entry *id3)
{
{
/* Use the trackname part of the id3 structure as a temporary buffer */
unsigned char* buf = (unsigned char *)id3->path;
unsigned long totalsamples;
@ -55,32 +55,32 @@ bool get_a52_metadata(int fd, struct mp3entry *id3)
return false;
}
if ((buf[0] != 0x0b) || (buf[1] != 0x77))
{
if ((buf[0] != 0x0b) || (buf[1] != 0x77))
{
logf("not an A52/AC3 file\n");
return false;
}
i = buf[4] & 0x3e;
if (i > 36)
if (i > 36)
{
logf("A52: Invalid frmsizecod: %d\n",i);
return false;
}
id3->bitrate = a52_bitrates[i >> 1];
id3->vbr = false;
id3->filesize = filesize(fd);
switch (buf[4] & 0xc0)
switch (buf[4] & 0xc0)
{
case 0x00:
case 0x00:
id3->frequency = 48000;
id3->bytesperframe=id3->bitrate * 2 * 2;
break;
case 0x40:
case 0x40:
id3->frequency = 44100;
id3->bytesperframe = a52_441framesizes[i];
break;

2
lib/rbcodec/metadata/aac.c
View file

@ -70,8 +70,6 @@ bool get_aac_metadata(int fd, struct mp3entry *entry)
unsigned char buf[5];
entry->title = NULL;
entry->tracknum = 0;
entry->discnum = 0;
entry->id3v1len = 0;
entry->id3v2len = getid3v2len(fd);
entry->first_frame_offset = entry->id3v2len;

2
lib/rbcodec/metadata/aiff.c
View file

@ -36,8 +36,6 @@
static void read_id3_tags(int fd, struct mp3entry* id3)
{
id3->tracknum = 0;
id3->discnum = 0;
setid3v2title(fd, id3);
}

12
lib/rbcodec/metadata/asf.c
View file

@ -24,6 +24,7 @@
#include <stdlib.h>
#include <ctype.h>
#include <inttypes.h>
#include <errno.h>
#include "platform.h"
#include "metadata.h"
@ -174,7 +175,7 @@ static int is_valid_utf16(const unsigned char *data, size_t length)
uint16_t second_last = data[length - 4] | (data[length - 3] << 8);
// Invalid if not preceded by a high surrogate
return second_last >= 0xD800 && second_last <= 0xDBFF;
return second_last >= 0xD800 && second_last <= 0xDBFF;
}
// If it's not a surrogate, it's valid
@ -473,7 +474,12 @@ static int asf_parse_header(int fd, struct mp3entry* id3,
if (type == 0) {
id3->track_string = id3buf;
asf_utf16LEdecode(fd, length, &id3buf, &id3buf_remaining);
id3->tracknum = atoi(id3->track_string);
if (strlen(id3->track_string)) {
char *p = NULL;
int tracknum = strtol(id3->track_string, &p, 0);
if (!(tracknum == 0 && (errno || *p)))
id3->tracknum = tracknum;
}
} else if ((type >=2) && (type <= 5)) {
id3->tracknum = asf_intdecode(fd, type, length);
} else {
@ -510,7 +516,7 @@ static int asf_parse_header(int fd, struct mp3entry* id3,
* "03 yy yy yy yy". xx is the size of the WM/Picture
* container in bytes. yy equals the raw data length of
* the embedded image.
*
*
* Also save position after this tag in file in case any parsing errors */
uint32_t after_pic_pos = lseek(fd, -4, SEEK_CUR) + 4 + length;

9
lib/rbcodec/metadata/id3tags.c
View file

@ -243,7 +243,12 @@ static int skip_unsynched(int fd, int len)
/* parse numeric value from string */
static int parsetracknum( struct mp3entry* entry, char* tag, int bufferpos )
{
entry->tracknum = atoi( tag );
if (strlen(tag)) {
char *p = NULL;
int tracknum = strtol(tag, &p, 0);
if (!(tracknum == 0 && (errno || *p)))
entry->tracknum = tracknum;
}
return bufferpos;
}
@ -826,7 +831,7 @@ void setid3v2title(int fd, struct mp3entry *entry)
return;
}
entry->id3version = version;
entry->tracknum = entry->year = entry->discnum = 0;
entry->year = entry->discnum = 0;
entry->title = entry->artist = entry->album = NULL; /* FIXME incomplete */
global_flags = header[5];

2
lib/rbcodec/metadata/metadata.c
View file

@ -452,6 +452,8 @@ bool get_metadata_ex(struct mp3entry* id3, int fd, const char* trackname, int fl
id3->has_embedded_cuesheet = false;
id3->embedded_cuesheet.pos = 0;
id3->tracknum = -1;
entry = &audio_formats[id3->codectype];
/* Load codec specific track tag information and confirm the codec type. */

8
lib/rbcodec/metadata/metadata_common.c
View file

@ -23,6 +23,7 @@
#include <stdlib.h>
#include <ctype.h>
#include <inttypes.h>
#include <errno.h>
#include "platform.h"
#include "metadata.h"
@ -315,7 +316,12 @@ long parse_tag(const char* name, char* value, struct mp3entry* id3,
if (((item == eTRACK && (type == TAGTYPE_APE)))
|| (item == eTRACKNUMBER && (type == TAGTYPE_VORBIS)))
{
id3->tracknum = atoi(value);
if (strlen(value)) {
char *p = NULL;
int tracknum = strtol(value, &p, 0);
if (!(tracknum == 0 && (errno || *p)))
id3->tracknum = tracknum;
}
p = &(id3->track_string);
}
else if (item == eDISCNUMBER || item == eDISC)

2
lib/rbcodec/metadata/mp3.c
View file

@ -166,8 +166,6 @@ bool get_mp3_metadata(int fd, struct mp3entry *entry)
entry->filesize = filesize(fd);
entry->id3v1len = getid3v1len(fd);
entry->id3v2len = getid3v2len(fd);
entry->tracknum = 0;
entry->discnum = 0;
if (entry->id3v2len)
setid3v2title(fd, entry);

124
lib/rbcodec/metadata/mp4.c
View file

@ -125,7 +125,7 @@ static unsigned int read_mp4_tag_string(int fd, int size_left, char** buffer,
if (bytes_read)
{
/* Do not overwrite already available metadata. Especially when reading
* tags with e.g. multiple genres / artists. This way only the first
* tags with e.g. multiple genres / artists. This way only the first
* of multiple entries is used, all following are dropped. */
if (*dest == NULL)
{
@ -143,11 +143,11 @@ static unsigned int read_mp4_tag_string(int fd, int size_left, char** buffer,
{
*dest = NULL;
}
return length;
}
static unsigned int read_mp4_atom(int fd, uint32_t* size,
static unsigned int read_mp4_atom(int fd, uint32_t* size,
uint32_t* type, uint32_t size_left)
{
read_uint32be(fd, size);
@ -155,7 +155,7 @@ static unsigned int read_mp4_atom(int fd, uint32_t* size,
if (*size == 1)
{
/* FAT32 doesn't support files this big, so something seems to
/* FAT32 doesn't support files this big, so something seems to
* be wrong. (64-bit sizes should only be used when required.)
*/
errno = EFBIG;
@ -173,7 +173,7 @@ static unsigned int read_mp4_atom(int fd, uint32_t* size,
{
size_left -= *size;
}
*size -= 8;
}
else
@ -181,7 +181,7 @@ static unsigned int read_mp4_atom(int fd, uint32_t* size,
*size = size_left;
size_left = 0;
}
return size_left;
}
@ -223,7 +223,7 @@ static bool read_mp4_esds(int fd, struct mp3entry* id3, uint32_t* size)
lseek(fd, 3, SEEK_CUR);
*size -= 3;
}
}
else
{
lseek(fd, 2, SEEK_CUR);
@ -242,11 +242,11 @@ static bool read_mp4_esds(int fd, struct mp3entry* id3, uint32_t* size)
{
return sbr;
}
lseek(fd, 13, SEEK_CUR); /* Skip audio type, bit rates, etc. */
read(fd, buf, 1);
*size -= 14;
if (*buf != 5) /* Verify DecSpecificInfoTag. */
{
return sbr;
@ -262,7 +262,7 @@ static bool read_mp4_esds(int fd, struct mp3entry* id3, uint32_t* size)
unsigned int length;
unsigned int index;
unsigned int type;
/* Read the (leading part of the) decoder config. */
length = read_mp4_length(fd, size);
length = MIN(length, *size);
@ -270,7 +270,7 @@ static bool read_mp4_esds(int fd, struct mp3entry* id3, uint32_t* size)
memset(buf, 0, sizeof(buf));
read(fd, buf, length);
*size -= length;
/* Maybe time to write a simple read_bits function... */
/* Decoder config format:
@ -287,14 +287,14 @@ static bool read_mp4_esds(int fd, struct mp3entry* id3, uint32_t* size)
{
id3->frequency = sample_rates[index];
}
if (type == 5)
{
unsigned int old_index = index;
sbr = true;
index = (bits >> 15) & 0xf; /* Frequency index - 4 bits */
if (index == 15)
{
/* 17 bits read so far... */
@ -305,7 +305,7 @@ static bool read_mp4_esds(int fd, struct mp3entry* id3, uint32_t* size)
{
id3->frequency = sample_rates[index];
}
if (old_index == index)
{
/* Downsampled SBR */
@ -318,7 +318,7 @@ static bool read_mp4_esds(int fd, struct mp3entry* id3, uint32_t* size)
/* We found an extensionAudioObjectType */
type = bits & 0x1f; /* Object type - 5 bits*/
bits = get_long_be(&buf[4]);
if (type == 5)
{
sbr = bits >> 31;
@ -327,7 +327,7 @@ static bool read_mp4_esds(int fd, struct mp3entry* id3, uint32_t* size)
if (sbr)
{
unsigned int old_index = index;
/* 1 bit read so far */
index = (bits >> 27) & 0xf; /* Frequency index - 4 bits */
@ -356,14 +356,14 @@ static bool read_mp4_esds(int fd, struct mp3entry* id3, uint32_t* size)
if (!sbr && !sbr_signaled && id3->frequency <= 24000)
{
/* As stated in libfaad/mp4.c AudioSpecificConfig2:
* no SBR signalled, this could mean either implicit signalling or no SBR in this file
* MPEG specification states: assume SBR on files with samplerate <= 24000 Hz
* no SBR signalled, this could mean either implicit signalling or no SBR in this file
* MPEG specification states: assume SBR on files with samplerate <= 24000 Hz
*/
id3->frequency *= 2;
sbr = true;
}
}
return sbr;
}
@ -389,7 +389,7 @@ static void read_mp4_tag_i_from_n(int fd, int *i, char** i_from_n_string, uint32
}
}
static bool read_mp4_tags(int fd, struct mp3entry* id3,
static bool read_mp4_tags(int fd, struct mp3entry* id3,
uint32_t size_left)
{
uint32_t size;
@ -402,18 +402,18 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
{
size_left = read_mp4_atom(fd, &size, &type, size_left);
/* DEBUGF("Tag atom: '%c%c%c%c' (%d bytes left)\n", type >> 24 & 0xff,
/* DEBUGF("Tag atom: '%c%c%c%c' (%d bytes left)\n", type >> 24 & 0xff,
type >> 16 & 0xff, type >> 8 & 0xff, type & 0xff, size); */
switch (type)
{
case MP4_cnam:
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
&id3->title);
break;
case MP4_cART:
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
&id3->artist);
break;
@ -426,12 +426,12 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
&id3->grouping);
break;
case MP4_calb:
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
&id3->album);
break;
case MP4_cwrt:
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
&id3->composer);
@ -446,7 +446,7 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
case MP4_cday:
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
&id3->year_string);
/* Try to parse it as a year, for the benefit of the database.
*/
if(id3->year_string)
@ -471,7 +471,7 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
id3->genre_string = id3_get_num_genre(betoh16(genre) - 1);
}
break;
case MP4_cgen:
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
&id3->genre_string);
@ -481,16 +481,20 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
read_mp4_tag_i_from_n(fd, &id3->discnum, &id3->disc_string, size, &buffer_left, &buffer);
break;
case MP4_trkn:
read_mp4_tag_i_from_n(fd, &id3->tracknum, &id3->track_string, size, &buffer_left, &buffer);
case MP4_trkn: {
char *p = NULL;
int tracknum = 0;
read_mp4_tag_i_from_n(fd, &tracknum, &id3->track_string, size, &buffer_left, &buffer);
if (!(tracknum == 0 && (errno || *p)))
id3->tracknum = tracknum;
break;
}
#ifdef HAVE_ALBUMART
case MP4_covr:
{
int pos = lseek(fd, 0, SEEK_CUR) + 16;
id3->albumart.type = AA_TYPE_UNKNOWN;
if (read_mp4_tag(fd, size, buffer, 8) >= 4)
{
@ -502,7 +506,7 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
{
id3->albumart.type = AA_TYPE_PNG;
}
if (id3->albumart.type != AA_TYPE_UNKNOWN)
{
id3->albumart.pos = pos;
@ -528,10 +532,10 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
size -= sub_size;
lseek(fd, 8, SEEK_CUR);
sub_size -= 12;
if (sub_size > sizeof(tag_name) - 1)
{
rd_ret = read(fd, tag_name, sizeof(tag_name) - 1);
rd_ret = read(fd, tag_name, sizeof(tag_name) - 1);
lseek(fd, sub_size - (sizeof(tag_name) - 1), SEEK_CUR);
sub_size = sizeof(tag_name) - 1;
}
@ -551,9 +555,9 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
if (tn_op == 0 && !cwrt) /*composer*/
{
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
&id3->composer);
}
}
else if (tn_op == 1) /*iTunSMPB*/
{
char value[TAG_VALUE_LENGTH];
@ -564,7 +568,7 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
read_mp4_tag_string(fd, size, &value_p, &length, &any);
id3->lead_trim = get_itunes_int32(value, 1);
id3->tail_trim = get_itunes_int32(value, 2);
DEBUGF("AAC: lead_trim %d, tail_trim %d\n",
DEBUGF("AAC: lead_trim %d, tail_trim %d\n",
id3->lead_trim, id3->tail_trim);
}
else if (tn_op == 2) /*musicbrainz track id*/
@ -574,9 +578,9 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
}
else if (tn_op == 3) /*album artist*/
{
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
read_mp4_tag_string(fd, size, &buffer, &buffer_left,
&id3->albumartist);
}
}
else
{
char* any = NULL;
@ -588,13 +592,13 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
/* Re-use the read buffer as the dest buffer... */
buffer -= length;
buffer_left += length;
parse_replaygain(tag_name, buffer, id3);
}
}
}
break;
default:
lseek(fd, size, SEEK_CUR);
break;
@ -605,7 +609,7 @@ static bool read_mp4_tags(int fd, struct mp3entry* id3,
return true;
}
static bool read_mp4_container(int fd, struct mp3entry* id3,
static bool read_mp4_container(int fd, struct mp3entry* id3,
uint32_t size_left)
{
uint32_t size = 0;
@ -613,16 +617,16 @@ static bool read_mp4_container(int fd, struct mp3entry* id3,
uint32_t handler = 0;
bool rc = true;
bool done = false;
do
{
size_left = read_mp4_atom(fd, &size, &type, size_left);
/* DEBUGF("Atom: '%c%c%c%c' (0x%08lx, %lu bytes left)\n",
/* DEBUGF("Atom: '%c%c%c%c' (0x%08lx, %lu bytes left)\n",
(int) ((type >> 24) & 0xff), (int) ((type >> 16) & 0xff),
(int) ((type >> 8) & 0xff), (int) (type & 0xff),
type, size); */
switch (type)
{
case MP4_ftyp:
@ -648,7 +652,7 @@ static bool read_mp4_container(int fd, struct mp3entry* id3,
rc = read_mp4_container(fd, id3, size);
size = 0;
break;
case MP4_ilst:
/* We need at least a size of 8 to read the next atom. */
if (handler == MP4_mdir && size>8)
@ -657,7 +661,7 @@ static bool read_mp4_container(int fd, struct mp3entry* id3,
size = 0;
}
break;
case MP4_minf:
if (handler == MP4_soun)
{
@ -665,22 +669,22 @@ static bool read_mp4_container(int fd, struct mp3entry* id3,
size = 0;
}
break;
case MP4_stsd:
lseek(fd, 8, SEEK_CUR);
size -= 8;
rc = read_mp4_container(fd, id3, size);
size = 0;
break;
case MP4_hdlr:
lseek(fd, 8, SEEK_CUR);
read_uint32be(fd, &handler);
size -= 12;
/* DEBUGF(" Handler '%c%c%c%c'\n", handler >> 24 & 0xff,
/* DEBUGF(" Handler '%c%c%c%c'\n", handler >> 24 & 0xff,
handler >> 16 & 0xff, handler >> 8 & 0xff,handler & 0xff); */
break;
case MP4_stts:
{
uint32_t entries;
@ -702,9 +706,9 @@ static bool read_mp4_container(int fd, struct mp3entry* id3,
read_uint32be(fd, &l);
/* Some AAC file use HE profile. In this case the number
* of output samples is doubled to a maximum of 2048
* samples per frame. This means that files which already
* report a frame size of 2048 in their header will not
* of output samples is doubled to a maximum of 2048
* samples per frame. This means that files which already
* report a frame size of 2048 in their header will not
* need any further special handling. */
if (id3->codectype==AFMT_MP4_AAC_HE && l<=1024)
{
@ -714,7 +718,7 @@ static bool read_mp4_container(int fd, struct mp3entry* id3,
id3->samples += (uint64_t) n * l;
}
size = 0;
}
break;
@ -807,14 +811,14 @@ static bool read_mp4_container(int fd, struct mp3entry* id3,
default:
break;
}
/* Skip final seek. */
if (!done)
{
lseek(fd, size, SEEK_CUR);
}
} while (rc && (size_left > 0) && (errno == 0) && !done);
return rc;
}
@ -824,8 +828,8 @@ bool get_mp4_metadata(int fd, struct mp3entry* id3)
id3->filesize = 0;
errno = 0;
if (read_mp4_container(fd, id3, filesize(fd)) && (errno == 0)
&& (id3->samples > 0) && (id3->frequency > 0)
if (read_mp4_container(fd, id3, filesize(fd)) && (errno == 0)
&& (id3->samples > 0) && (id3->frequency > 0)
&& (id3->filesize > 0))
{
if (id3->codectype == AFMT_UNKNOWN)

2
lib/rbcodec/metadata/tta.c
View file

@ -56,8 +56,6 @@ static void read_id3_tags(int fd, struct mp3entry* id3)
id3->title = NULL;
id3->filesize = filesize(fd);
id3->id3v2len = getid3v2len(fd);
id3->tracknum = 0;
id3->discnum = 0;
id3->vbr = false; /* All TTA files are CBR */
/* first get id3v2 tags. if no id3v2 tags ware found, get id3v1 tags */

2
lib/rbcodec/test/warble.c
View file

@ -732,7 +732,7 @@ static void print_mp3entry(const struct mp3entry *id3, FILE *f)
if (id3->album) fprintf(f, "Album: %s\n", id3->album);
if (id3->genre_string) fprintf(f, "Genre: %s\n", id3->genre_string);
if (id3->disc_string || id3->discnum) fprintf(f, "Disc: %s (%d)\n", id3->disc_string, id3->discnum);
if (id3->track_string || id3->tracknum) fprintf(f, "Track: %s (%d)\n", id3->track_string, id3->tracknum);
if (id3->track_string || id3->tracknum >= 0) fprintf(f, "Track: %s (%d)\n", id3->track_string, id3->tracknum);
if (id3->year_string || id3->year) fprintf(f, "Year: %s (%d)\n", id3->year_string, id3->year);
if (id3->composer) fprintf(f, "Composer: %s\n", id3->composer);
if (id3->comment) fprintf(f, "Comment: %s\n", id3->comment);