From 7af45378c3d8471682f07bc7075b8534e3f03773 Mon Sep 17 00:00:00 2001 From: "Timothy B. Terriberry" Date: Wed, 24 Apr 2013 21:52:52 -0700 Subject: [PATCH] Use FLAC's picture selection for Vorbis/Opus. When multiple pictures are embedded in a file, the FLAC demuxer prioritizes them based on the picture type. This ports the same algorithm over so it can be used by Vorbis and Opus. It also extends its behavior for FLAC to work with both pictures embedded in VORBIS_COMMENT blocks and in normal PICTURE blocks. This also plugs a memory leak in vorbis_ParseComment() when parsing METADATA_BLOCK_PICTURE tags. Signed-off-by: Jean-Baptiste Kempf --- modules/demux/flac.c | 28 +++++------------------- modules/demux/ogg.c | 12 +++++++++++ modules/demux/vorbis.h | 39 +++++++++++++++++++++++++--------- modules/meta_engine/taglib.cpp | 8 +++++-- 4 files changed, 52 insertions(+), 35 deletions(-) diff --git a/modules/demux/flac.c b/modules/demux/flac.c index c18b5a9dbe..2c16272d9e 100644 --- a/modules/demux/flac.c +++ b/modules/demux/flac.c @@ -574,40 +574,22 @@ static void ParseComment( demux_t *p_demux, const uint8_t *p_data, int i_data ) if( i_data < 4 ) return; - vorbis_ParseComment( &p_sys->p_meta, &p_data[4], i_data - 4, NULL, NULL, NULL, NULL ); - + vorbis_ParseComment( &p_sys->p_meta, &p_data[4], i_data - 4, + &p_sys->i_attachments, &p_sys->attachments, + &p_sys->i_cover_score, &p_sys->i_cover_idx, NULL, NULL ); } static void ParsePicture( demux_t *p_demux, const uint8_t *p_data, int i_data ) { - static const int pi_cover_score[] = { - 0, /* other */ - 2, 1, /* icons */ - 10, /* front cover */ - 9, /* back cover */ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 6, /* movie/video screen capture */ - 0, - 7, /* Illustration */ - 8, /* Band/Artist logotype */ - 0, /* Publisher/Studio */ - }; demux_sys_t *p_sys = p_demux->p_sys; - int i_type; i_data -= 4; p_data += 4; - input_attachment_t *p_attachment = ParseFlacPicture( p_data, i_data, p_sys->i_attachments, &i_type ); + input_attachment_t *p_attachment = ParseFlacPicture( p_data, i_data, + p_sys->i_attachments, &p_sys->i_cover_score, &p_sys->i_cover_idx ); if( p_attachment == NULL ) return; TAB_APPEND( p_sys->i_attachments, p_sys->attachments, p_attachment ); - - if( i_type >= 0 && (unsigned int)i_type < sizeof(pi_cover_score)/sizeof(pi_cover_score[0]) && - p_sys->i_cover_score < pi_cover_score[i_type] ) - { - p_sys->i_cover_idx = p_sys->i_attachments-1; - p_sys->i_cover_score = pi_cover_score[i_type]; - } } diff --git a/modules/demux/ogg.c b/modules/demux/ogg.c index 185077126b..8afaeb6ba8 100644 --- a/modules/demux/ogg.c +++ b/modules/demux/ogg.c @@ -1826,9 +1826,21 @@ static void Ogg_ExtractXiphMeta( demux_t *p_demux, const void *p_headers, unsign /* TODO how to handle multiple comments properly ? */ if( i_count >= 2 && pi_size[1] > i_skip ) + { + int i_cover_score = 0; + int i_cover_idx = 0; vorbis_ParseComment( &p_ogg->p_meta, (uint8_t*)pp_data[1] + i_skip, pi_size[1] - i_skip, &p_ogg->i_attachments, &p_ogg->attachments, + &i_cover_score, &i_cover_idx, &p_ogg->i_seekpoints, &p_ogg->pp_seekpoints ); + if( p_ogg->p_meta != NULL && i_cover_idx < p_ogg->i_attachments ) + { + char psz_url[128]; + snprintf( psz_url, sizeof(psz_url), "attachment://%s", + p_ogg->attachments[i_cover_idx]->psz_name ); + vlc_meta_Set( p_ogg->p_meta, vlc_meta_ArtworkURL, psz_url ); + } + } if( p_ogg->i_seekpoints > 1 ) { diff --git a/modules/demux/vorbis.h b/modules/demux/vorbis.h index ccb3d7f355..d42163580f 100644 --- a/modules/demux/vorbis.h +++ b/modules/demux/vorbis.h @@ -25,9 +25,24 @@ #include #include -static input_attachment_t* ParseFlacPicture( const uint8_t *p_data, int i_data, int i_attachments, int *i_type ) +static input_attachment_t* ParseFlacPicture( const uint8_t *p_data, int i_data, + int i_attachments, int *i_cover_score, int *i_cover_idx ) { + static const char pi_cover_score[] = { + 0, /* other */ + 2, 1, /* icons */ + 10, /* front cover */ + 9, /* back cover */ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 6, /* movie/video screen capture */ + 0, + 7, /* Illustration */ + 8, /* Band/Artist logotype */ + 0, /* Publisher/Studio */ + }; + int i_len; + int i_type; char *psz_mime = NULL; char psz_name[128]; char *psz_description = NULL; @@ -37,7 +52,7 @@ static input_attachment_t* ParseFlacPicture( const uint8_t *p_data, int i_data, return NULL; #define RM(x) do { i_data -= (x); p_data += (x); } while(0) - *i_type = GetDWBE( p_data ); RM(4); + i_type = GetDWBE( p_data ); RM(4); i_len = GetDWBE( p_data ); RM(4); if( i_len < 0 || i_data < i_len + 4 ) @@ -54,7 +69,7 @@ static input_attachment_t* ParseFlacPicture( const uint8_t *p_data, int i_data, goto error; /* printf( "Picture type=%d mime=%s description='%s' file length=%d\n", - *i_type, psz_mime, psz_description, i_len ); */ + i_type, psz_mime, psz_description, i_len ); */ snprintf( psz_name, sizeof(psz_name), "picture%d", i_attachments ); if( !strcasecmp( psz_mime, "image/jpeg" ) ) @@ -65,6 +80,13 @@ static input_attachment_t* ParseFlacPicture( const uint8_t *p_data, int i_data, p_attachment = vlc_input_attachment_New( psz_name, psz_mime, psz_description, p_data, i_data ); + if( i_type >= 0 && (unsigned int)i_type < sizeof(pi_cover_score)/sizeof(pi_cover_score[0]) && + *i_cover_score < pi_cover_score[i_type] ) + { + *i_cover_idx = i_attachments; + *i_cover_score = pi_cover_score[i_type]; + } + error: free( psz_mime ); free( psz_description ); @@ -74,11 +96,11 @@ error: static inline void vorbis_ParseComment( vlc_meta_t **pp_meta, const uint8_t *p_data, int i_data, int *i_attachments, input_attachment_t ***attachments, + int *i_cover_score, int *i_cover_idx, int *i_seekpoint, seekpoint_t ***ppp_seekpoint ) { int n; int i_comment; - int i_attach = 0; seekpoint_t *sk = NULL; if( i_data < 8 ) @@ -175,16 +197,13 @@ static inline void vorbis_ParseComment( vlc_meta_t **pp_meta, if( attachments == NULL ) continue; - int i; uint8_t *p_picture; size_t i_size = vlc_b64_decode_binary( &p_picture, &psz_comment[strlen("METADATA_BLOCK_PICTURE=")]); - input_attachment_t *p_attachment = ParseFlacPicture( p_picture, i_size, i_attach, &i ); + input_attachment_t *p_attachment = ParseFlacPicture( p_picture, + i_size, *i_attachments, i_cover_score, i_cover_idx ); + free( p_picture ); if( p_attachment ) { - char psz_url[128]; - snprintf( psz_url, sizeof(psz_url), "attachment://%s", p_attachment->psz_name ); - vlc_meta_Set( p_meta, vlc_meta_ArtworkURL, psz_url ); - i_attach++; TAB_APPEND_CAST( (input_attachment_t**), *i_attachments, *attachments, p_attachment ); } diff --git a/modules/meta_engine/taglib.cpp b/modules/meta_engine/taglib.cpp index 49561d1853..76c878535c 100644 --- a/modules/meta_engine/taglib.cpp +++ b/modules/meta_engine/taglib.cpp @@ -467,9 +467,13 @@ static void ReadMetaFromXiph( Ogg::XiphComment* tag, demux_meta_t* p_demux_meta, return; uint8_t *p_data; - int type; + int i_cover_score; + int i_cover_idx; int i_data = vlc_b64_decode_binary( &p_data, art_list[0].toCString(true) ); - p_attachment = ParseFlacPicture( p_data, i_data, 0, &type ); + i_cover_score = i_cover_idx = 0; + /* TODO: Use i_cover_score / i_cover_idx to select the picture. */ + p_attachment = ParseFlacPicture( p_data, i_data, 0, + &i_cover_score, &i_cover_idx ); } TAB_INIT( p_demux_meta->i_attachments, p_demux_meta->attachments ); -- 2.39.2