Skip to content

Commit

Permalink
[scan] Fixup lyrics changes
Browse files Browse the repository at this point in the history
  • Loading branch information
ejurgensen committed Oct 19, 2023
1 parent f3ba450 commit 2772d21
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 70 deletions.
2 changes: 2 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ OWNTONE_MODULES_CHECK([OWNTONE], [LIBAV],
[libavutil/avutil.h])
OWNTONE_CHECK_DECLS([avformat_network_init],
[libavformat/avformat.h])
OWNTONE_CHECK_DECLS([av_dict_iterate],
[libavutil/dict.h])
])

AC_CHECK_SIZEOF([void *])
Expand Down
121 changes: 51 additions & 70 deletions src/library/filescanner_ffmpeg.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,23 @@
#include "http.h"
#include "conffile.h"

// From libavutil 57.37.100
#ifndef HAVE_DECL_AV_DICT_ITERATE
# define av_dict_iterate(dict, entry) av_dict_get((dict), "", (entry), AV_DICT_IGNORE_SUFFIX)
#endif

/* Mapping between the metadata name(s) and the offset
* of the equivalent metadata field in struct media_file_info */
struct metadata_map {
char *key;
int as_int;
size_t offset;
int (*handler_function)(struct media_file_info *, char *);
int (*handler_function)(struct media_file_info *, const char *);
};

// Used for passing errors to DPRINTF (can't count on av_err2str being present)
static char errbuf[64];

static int lyricsindex = -1;

static inline char *
err2str(int errnum)
{
Expand All @@ -59,7 +62,7 @@ err2str(int errnum)
}

static int
parse_genre(struct media_file_info *mfi, char *genre_string)
parse_genre(struct media_file_info *mfi, const char *genre_string)
{
char **genre = (char**)((char *) mfi + mfi_offsetof(genre));
char *ptr;
Expand All @@ -80,27 +83,32 @@ parse_genre(struct media_file_info *mfi, char *genre_string)
}

static int
parse_slash_separated_ints(char *string, uint32_t *firstval, uint32_t *secondval)
parse_slash_separated_ints(const char *string, uint32_t *firstval, uint32_t *secondval)
{
int numvals = 0;
char buf[64];
char *ptr;

ptr = strchr(string, '/');
// dict.h: "The returned entry key or value must not be changed, or it will
// cause undefined behavior" -> so we must make a copy
snprintf(buf, sizeof(buf), "%s", string);

ptr = strchr(buf, '/');
if (ptr)
{
*ptr = '\0';
if (safe_atou32(ptr + 1, secondval) == 0)
numvals++;
}

if (safe_atou32(string, firstval) == 0)
if (safe_atou32(buf, firstval) == 0)
numvals++;

return numvals;
}

static int
parse_track(struct media_file_info *mfi, char *track_string)
parse_track(struct media_file_info *mfi, const char *track_string)
{
uint32_t *track = (uint32_t *) ((char *) mfi + mfi_offsetof(track));
uint32_t *total_tracks = (uint32_t *) ((char *) mfi + mfi_offsetof(total_tracks));
Expand All @@ -109,7 +117,7 @@ parse_track(struct media_file_info *mfi, char *track_string)
}

static int
parse_disc(struct media_file_info *mfi, char *disc_string)
parse_disc(struct media_file_info *mfi, const char *disc_string)
{
uint32_t *disc = (uint32_t *) ((char *) mfi + mfi_offsetof(disc));
uint32_t *total_discs = (uint32_t *) ((char *) mfi + mfi_offsetof(total_discs));
Expand All @@ -118,7 +126,7 @@ parse_disc(struct media_file_info *mfi, char *disc_string)
}

static int
parse_date(struct media_file_info *mfi, char *date_string)
parse_date(struct media_file_info *mfi, const char *date_string)
{
char year_string[32];
uint32_t *year = (uint32_t *) ((char *) mfi + mfi_offsetof(year));
Expand Down Expand Up @@ -154,7 +162,7 @@ parse_date(struct media_file_info *mfi, char *date_string)
}

static int
parse_albumid(struct media_file_info *mfi, char *id_string)
parse_albumid(struct media_file_info *mfi, const char *id_string)
{
// Already set by a previous tag that we give higher priority
if (mfi->songalbumid)
Expand Down Expand Up @@ -280,92 +288,65 @@ static const struct metadata_map md_map_id3[] =
{ NULL, 0, 0, NULL }
};

/* TODO: If the md_map was sorted, the search would be O(log N) instead of O(N) */
static int
match_metadata(const char *key, const struct metadata_map *md_map)
{
int i;

for (i = 0; md_map[i].key != NULL; i++)
{
if (strcmp(key, md_map[i].key) == 0)
return i;
}
return -1;
}


static int
iterate_metadata(struct media_file_info *mfi, const AVDictionaryEntry *mdt, const struct metadata_map *md_map)
extract_metadata_from_kv(struct media_file_info *mfi, const char *key, const char *value, const struct metadata_map *md_map)
{
char **strval;
uint32_t *intval;
int i;

if ((mdt->value == NULL) || (strlen(mdt->value) == 0))
if ((value == NULL) || (strlen(value) == 0))
return 0;

if (strncmp(mdt->key, "lyrics-", sizeof("lyrics-") - 1) == 0)
i = lyricsindex;
else
i = match_metadata(mdt->key, md_map);
if (strncmp(key, "lyrics-", sizeof("lyrics-") - 1) == 0)
key = "lyrics";

if (i == -1)
return 0;
for (i = 0; md_map[i].key != NULL; i++)
{
if (strcmp(key, md_map[i].key) == 0)
break;
}

if (md_map[i].key == NULL)
return 0; // Not found in map

if (md_map[i].handler_function)
return md_map[i].handler_function(mfi, mdt->value);
return md_map[i].handler_function(mfi, value);

if (!md_map[i].as_int)
{
strval = (char **) ((char *) mfi + md_map[i].offset);

if (*strval == NULL)
*strval = strdup(mdt->value);
if (*strval != NULL)
return 0;

*strval = strdup(value);
}
else
{
intval = (uint32_t *) ((char *) mfi + md_map[i].offset);

if (*intval == 0)
{
if (safe_atou32(mdt->value, intval) < 0)
return 0;
}
if (*intval != 0)
return 0;

if (safe_atou32(value, intval) < 0)
return 0;
}

return 1;
}

static int
extract_metadata_core(struct media_file_info *mfi, AVDictionary *md, const struct metadata_map *md_map)
extract_metadata_from_dict(struct media_file_info *mfi, AVDictionary *md, const struct metadata_map *md_map)
{
const AVDictionaryEntry *mdt;
int mdcount;

#if 0
/* Dump all the metadata reported by ffmpeg */
mdt = NULL;
while ((mdt = av_dict_get(md, "", mdt, AV_DICT_IGNORE_SUFFIX)) != NULL)
DPRINTF(E_DBG, L_SCAN, " -> %s = %s\n", mdt->key, mdt->value);
#endif

mdcount = 0;
const AVDictionaryEntry *mdt = NULL;
int mdcount = 0;

/* Cache this search once to avoid doing it per metadata key */
if (lyricsindex == -1)
lyricsindex = match_metadata("lyrics", md_map);

/* Extract lyrics if any found.
FFMPEG creates a metadata key that's embedding the language code in it, like 'lyrics-eng' or 'lyrics-XXX'
So it's not possible to query this key directly except by brute-forcing all languages.
Instead, we are reversing the metadata searching algorithm to query all metadata key that FFMPEG fetched
and matching them against our own map. This is the most efficient method to search it without having 2 pass
on the KV store */
mdt = av_dict_get(md, "", NULL, AV_DICT_IGNORE_SUFFIX);
while (mdt != NULL)
while ((mdt = av_dict_iterate(md, mdt)))
{
mdcount += iterate_metadata(mfi, mdt, md_map);
mdt = av_dict_get(md, "", mdt, AV_DICT_IGNORE_SUFFIX);
DPRINTF(E_SPAM, L_SCAN, " -> %s = %s\n", mdt->key, mdt->value);
mdcount += extract_metadata_from_kv(mfi, mdt->key, mdt->value, md_map);
}

return mdcount;
Expand All @@ -381,23 +362,23 @@ extract_metadata(struct media_file_info *mfi, AVFormatContext *ctx, AVStream *au

if (ctx->metadata)
{
ret = extract_metadata_core(mfi, ctx->metadata, md_map);
ret = extract_metadata_from_dict(mfi, ctx->metadata, md_map);
mdcount += ret;

DPRINTF(E_DBG, L_SCAN, "Picked up %d tags from file metadata\n", ret);
}

if (audio_stream->metadata)
{
ret = extract_metadata_core(mfi, audio_stream->metadata, md_map);
ret = extract_metadata_from_dict(mfi, audio_stream->metadata, md_map);
mdcount += ret;

DPRINTF(E_DBG, L_SCAN, "Picked up %d tags from audio stream metadata\n", ret);
}

if (video_stream && video_stream->metadata)
{
ret = extract_metadata_core(mfi, video_stream->metadata, md_map);
ret = extract_metadata_from_dict(mfi, video_stream->metadata, md_map);
mdcount += ret;

DPRINTF(E_DBG, L_SCAN, "Picked up %d tags from video stream metadata\n", ret);
Expand Down

0 comments on commit 2772d21

Please sign in to comment.