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 <jb@videolan.org>

diff --git a/modules/demux/flac.c b/modules/demux/flac.c
index c18b5a9dbe29672cd04a6e7b41494000730828b5..2c16272d9e84c3f396fe826cee2fc2e65b488dd9 100644 (file)
--- 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 185077126b344e34ac006d7ca683ea7bcb7f621e..8afaeb6ba81d1cc36b0123e7f937247d6546d972 100644 (file)
--- 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 ccb3d7f355c5d26265b70ff03de5d68cdc0b89d5..d42163580ffb745ba68fc348c8fc1010fcb8beef 100644 (file)
--- a/modules/demux/vorbis.h
+++ b/modules/demux/vorbis.h
@@ -25,9 +25,24 @@
 #include <vlc_strings.h>
 #include <vlc_input.h>
 
-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 49561d185365e443b140cf151156292b0213ddb6..76c878535c495c2af4192c47b79ead6caaae4cb0 100644 (file)
--- 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 );