Skip to content

Commit

Permalink
music_id3tag.c: skipping of lyrics3 tags
Browse files Browse the repository at this point in the history
  • Loading branch information
Wohlstand committed Dec 5, 2019
1 parent d2d2b80 commit 178e5f3
Showing 1 changed file with 149 additions and 1 deletion.
150 changes: 149 additions & 1 deletion src/codecs/music_id3tag.c
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,120 @@ static SDL_bool parse_id3v2(Mix_MusicMetaTags *out_tags, SDL_RWops *src, Sint64
}


/********************************************************
* Lyrics3 skip *
********************************************************/

/* The maximum length of the lyrics is 5100 bytes. (Regard to a specification) */
#define LYRICS3v1_SEARCH_BUFFER 5120

#define LYRICS3v1_HEAD_SIZE 11
#define LYRICS3v1_TAIL_SIZE 9
#define LYRICS3v2_TAG_SIZE_VALUE 6


static SDL_INLINE SDL_bool is_lyrics3(const Uint8 *data, size_t length)
{
if (length < LYRICS3v1_TAIL_SIZE || (SDL_memcmp(data,"LYRICSEND", 9) != 0 && SDL_memcmp(data,"LYRICS200", 9) != 0)) {
return SDL_FALSE;
}
return SDL_TRUE;
}

static long lyrics3_skip(Sint64 tag_end_at, Sint64 begin_pos, SDL_RWops *src)
{
Sint64 file_size, pos_begin, pos_end;
size_t read_size;
char buffer[LYRICS3v1_SEARCH_BUFFER + 1];
char *cur, *end = (buffer + LYRICS3v1_SEARCH_BUFFER);
long len = 0;

file_size = SDL_RWsize(src);
if (file_size < 0) {
SDL_RWseek(src, begin_pos, RW_SEEK_SET);
return 0; /* Invalid tag */

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

return -1 here for consistency with others

}

SDL_RWseek(src, -tag_end_at, RW_SEEK_END);

SDL_RWseek(src, -LYRICS3v1_TAIL_SIZE, RW_SEEK_CUR);

This comment has been minimized.

Copy link
@Wohlstand

Wohlstand Dec 6, 2019

Author Member

A bad thing...
However, it was simplified in a next commit.

read_size = SDL_RWread(src, buffer, 1, LYRICS3v1_TAIL_SIZE);

/* Find and validate borders of the lyrics tag */
if (SDL_memcmp(buffer, "LYRICSEND", LYRICS3v1_TAIL_SIZE) == 0) { /* Lyrics3 v1 */
/* Lyrics3 v1.00 tag
* http://id3.org/Lyrics3 */
pos_end = SDL_RWtell(src);
if (pos_end > LYRICS3v1_SEARCH_BUFFER) {
SDL_RWseek(src, -LYRICS3v1_SEARCH_BUFFER, RW_SEEK_CUR);
} else {
SDL_RWseek(src, 0, RW_SEEK_SET);
}
pos_begin = SDL_RWtell(src);

read_size = SDL_RWread(src, buffer, 1, (size_t)(pos_end - pos_begin));
end = (buffer + read_size);

/* Find the lyrics begin tag... */
for (cur = buffer; cur != end; cur++) {
if (SDL_memcmp(cur, "LYRICSBEGIN", LYRICS3v1_HEAD_SIZE) == 0) {
/* Calculate a full length of a tag */
len = (long)(end - cur);
break;
}
}

if (cur == end) {
SDL_RWseek(src, begin_pos, RW_SEEK_SET);
return 0;

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

.. likewise

}

} else if (SDL_memcmp(buffer, "LYRICS200", 9) == 0) { /* Lyrics3 v2 */
/* Lyrics3 v2.00 tag
* http://id3.org/Lyrics3v2 */
pos_end = SDL_RWtell(src);
if (pos_end > LYRICS3v2_TAG_SIZE_VALUE + LYRICS3v1_TAIL_SIZE) {
SDL_RWseek(src, -(LYRICS3v2_TAG_SIZE_VALUE + LYRICS3v1_TAIL_SIZE), RW_SEEK_CUR);
} else {
SDL_RWseek(src, begin_pos, RW_SEEK_SET);
return 0; /* Invalid tag */

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

.. likewise

}

read_size = SDL_RWread(src, buffer, 1, LYRICS3v2_TAG_SIZE_VALUE);
if (read_size < LYRICS3v2_TAG_SIZE_VALUE) {
SDL_RWseek(src, begin_pos, RW_SEEK_SET);
return 0; /* Invalid tag */

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

.. likewise

}
buffer[read_size] = '\0';

len = SDL_strtol(buffer, NULL, 10);
if (len == 0) {
SDL_RWseek(src, begin_pos, RW_SEEK_SET);
return 0; /* Invalid tag */

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

.. likewise

}

len += LYRICS3v2_TAG_SIZE_VALUE + LYRICS3v1_TAIL_SIZE;

if (pos_end > len) {
SDL_RWseek(src, (pos_end - len), RW_SEEK_SET);
read_size = SDL_RWread(src, buffer, 1, LYRICS3v1_HEAD_SIZE);
if (read_size < LYRICS3v1_HEAD_SIZE) {
SDL_RWseek(src, begin_pos, RW_SEEK_SET);
return 0; /* Invalid tag */

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

.. likewise

}

if (SDL_memcmp(buffer, "LYRICSBEGIN", LYRICS3v1_HEAD_SIZE) != 0) {
SDL_RWseek(src, begin_pos, RW_SEEK_SET);
return 0; /* Invalid tag */

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

.. likewise

}
}
}

SDL_RWseek(src, begin_pos, RW_SEEK_SET);
return len;
}


/********************************************************
* APE v1 and v2 *
********************************************************/
Expand Down Expand Up @@ -630,7 +744,7 @@ static SDL_bool parse_ape(Mix_MusicMetaTags *out_tags, SDL_RWops *src, Sint64 be

SDL_RWseek(src, begin_pos, RW_SEEK_SET);

return SDL_TRUE;
return 0;
}

int id3tag_fetchTags(Mix_MusicMetaTags *out_tags, SDL_RWops *src, Id3TagLengthStrip *file_edges)
Expand Down Expand Up @@ -715,6 +829,23 @@ int id3tag_fetchTags(Mix_MusicMetaTags *out_tags, SDL_RWops *src, Id3TagLengthSt
goto end;
}

/* Skip the lyrics tag that is going before ID3v1 tag */

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

Here is the thing: who dictates the order of eccentric / unofficial tags here? We know id3v1ext should be before id3v1, but ape and lyrics3 things? ugh. Possibly don't support them unless we have real files from out there, or: just remove the duplicated ape and lyrics3 blocks from here.

This comment has been minimized.

Copy link
@Wohlstand

Wohlstand Dec 6, 2019

Author Member

In a specification, lyrics3 can appear only at end of the file with or without leading ID3v1 tag.
About duplicated code: maybe make one another function for APE to don't duplicate the code?
Exactly, the first time we should dig up some files in a wild to confirm the thing.

This comment has been minimized.

Copy link
@Wohlstand

Wohlstand Dec 6, 2019

Author Member

Why duplicates of lyrics3? First handles "after-id3v1", second handles "lonely".

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

In a specification, lyrics3 can appear only at end of the file with or without leading ID3v1 tag.

Then the decision is: before id3v1:

  • if we have lyrics3, we don't have anything else (if that would
    be the case, you need a goto end there)
  • if we have id3v1ext, we don't have anything else (we do have a
    goto end in there)

Each of these decisions are by 'interpreting' the specs

This comment has been minimized.

Copy link
@Wohlstand

Wohlstand Dec 6, 2019

Author Member

I guess, you right, I should goto end when catching each of these tags...

SDL_RWseek(src, -(tail_size + LYRICS3v1_TAIL_SIZE), RW_SEEK_END);
readsize = SDL_RWread(src, in_buffer, 1, LYRICS3v1_TAIL_SIZE);
SDL_RWseek(src, begin_pos, RW_SEEK_SET);

if (is_lyrics3(in_buffer, readsize)) {
len = lyrics3_skip(tail_size, begin_pos, src);
if (len > 0) {

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

if (len < 0) return -1; for consistency with others

file_size -= (size_t)len;
tail_size += len;
if (file_edges) {
file_edges->end += len;
}
}
}

/* Skip the APE tag */
SDL_RWseek(src, -(tail_size + APE_HEADER_SIZE), RW_SEEK_END);
readsize = SDL_RWread(src, in_buffer, 1, APE_HEADER_SIZE);
SDL_RWseek(src, begin_pos, RW_SEEK_SET);
Expand Down Expand Up @@ -780,6 +911,23 @@ int id3tag_fetchTags(Mix_MusicMetaTags *out_tags, SDL_RWops *src, Id3TagLengthSt
}
}

/* Skip the lyrics tag at end of file */

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

Same question here: who dictates the order of eccentric / unofficial tags here? For this part, I don't have a suggestion.

This comment has been minimized.

Copy link
@Wohlstand

Wohlstand Dec 6, 2019

Author Member

Therefore looks like a reason to allow any order of these tags as, I guess, different software would write them with a different order. 🤔
I have some ideas but will try it tomorrow.

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

In theory maybe, but do we have any such abominations?

This comment has been minimized.

Copy link
@Wohlstand

Wohlstand Dec 6, 2019

Author Member

In theory maybe, but do we have any such abominations?

Personally I didn't saw such files with a mixture of different tag formats and I guess, they are conflicting by a spec and incompatible with a rest of software.

This comment has been minimized.

Copy link
@sezero

sezero Dec 6, 2019

Collaborator

In theory maybe, but do we have any such abominations?

Personally I didn't saw such files with a mixture of different tag formats and I guess,
they are conflicting by a spec and incompatible with a rest of software.

That is exactly my point

if (file_size >= LYRICS3v1_TAIL_SIZE) {
SDL_RWseek(src, -(tail_size + LYRICS3v1_TAIL_SIZE), RW_SEEK_END);
readsize = SDL_RWread(src, in_buffer, 1, LYRICS3v1_TAIL_SIZE);
SDL_RWseek(src, begin_pos, RW_SEEK_SET);
if (is_lyrics3(in_buffer, readsize)) {
len = lyrics3_skip(tail_size, begin_pos, src);
if (len > 0) {
file_size -= (size_t)len;
tail_size += len;
if (file_edges) {
file_edges->end += len;
}
}
}
}

ape: /* APE tag may be at the end: read the footer */
if (file_size >= APE_HEADER_SIZE) {
SDL_RWseek(src, -(tail_size + APE_HEADER_SIZE), RW_SEEK_END);
Expand Down

0 comments on commit 178e5f3

Please sign in to comment.