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>

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];
-    }
 }
 

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 )
     {

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 );
             }

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 );