[vlc-commits] [Git][videolan/vlc][master] 11 commits: cdrom: simplify the CDROM_READ_TOC_EX structure init

Jean-Baptiste Kempf (@jbk) gitlab at videolan.org
Mon Sep 13 12:12:44 UTC 2021



Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC


Commits:
2101c252 by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdrom: simplify the CDROM_READ_TOC_EX structure init

We know we want to read 4 bytes exactly, which always higher than
MINIMUM_CDROM_READ_TOC_EX_SIZE (2).

- - - - -
8b2d14a9 by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdrom: don't try to read CD-Text information if we only have 4 bytes

It will end up returning -1 but we can avoid going further for no reason.

- - - - -
78cea805 by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdrom: remove always false CD-Text test

CDTEXT_MAX_TRACKS is 0x7F, so we can never get more, since the number of track
is already read using 0x7f. The real check is the upper bits check in the next
line (extension flag).

- - - - -
d2f65642 by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdrom: discard empty CD-Text fields

Something some empty values are set in the file.

- - - - -
54598f7e by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdrom: add the CD-Text album artist field for each track

- - - - -
52eed4d9 by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdrom: use an enum for CD-Text pack types

- - - - -
6f5f25e6 by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdrom: use the CD-Text EAN/ISRC code as extra metadata

The names are similar to the ones found in Vorbis.

http://age.hobba.nl/audio/mirroredpages/ogg-tagging.html

- - - - -
3ed90beb by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdrom: add support for CD-Text extra metadata

The names are similar to the ones found in Vorbis.

http://age.hobba.nl/audio/mirroredpages/ogg-tagging.html

- - - - -
e086dc3b by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdda: favor track metadata over album metadata from CD-Text

The same variable may be set, but we should use the one from the tracks.

Only genre and description may not be set per track. The other are written
properly on each track.

- - - - -
5fbe7c39 by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdda: forward the CD-Text album artist to the input item

- - - - -
84dbe807 by Steve Lhomme at 2021-09-13T11:53:54+00:00
cdda: forward extra metadata from CD-Text to the input item

- - - - -


2 changed files:

- modules/access/cdda.c
- modules/access/vcd/cdrom.c


Changes:

=====================================
modules/access/cdda.c
=====================================
@@ -739,9 +739,14 @@ static int ReadDir(stream_t *access, input_item_node_t *node)
 
         const char *title = NULL;
         const char *artist = NULL;
+        const char *album_artist = NULL;
         const char *album = NULL;
         const char *genre = NULL;
         const char *description = NULL;
+        const char *author = NULL;
+        const char *composer = NULL;
+        const char *arranger = NULL;
+        const char *isrc = NULL;
         int year = 0;
 
 #ifdef HAVE_LIBCDDB
@@ -765,21 +770,27 @@ static int ReadDir(stream_t *access, input_item_node_t *node)
         if(sys->cdtextc > 0)
         {
             const vlc_meta_t *m;
-            if (i + 1 < sys->cdtextc && (m = sys->cdtextv[i + 1]) != NULL)
+
+            /* Album CDtext data */
+            if ((m = sys->cdtextv[0]) != NULL)
             {
-                ON_EMPTY(title,       vlc_meta_Get(m, vlc_meta_Title));
-                ON_EMPTY(artist,      vlc_meta_Get(m, vlc_meta_Artist));
                 ON_EMPTY(genre,       vlc_meta_Get(m, vlc_meta_Genre));
                 ON_EMPTY(description, vlc_meta_Get(m, vlc_meta_Description));
             }
 
-            /* Album CDtext data */
-            if ((m = sys->cdtextv[0]) != NULL)
+            if (i + 1 < sys->cdtextc && (m = sys->cdtextv[i + 1]) != NULL)
             {
+                ON_EMPTY(title,       vlc_meta_Get(m, vlc_meta_Title));
                 ON_EMPTY(artist,      vlc_meta_Get(m, vlc_meta_Artist));
+                ON_EMPTY(album_artist,vlc_meta_Get(m, vlc_meta_AlbumArtist));
                 ON_EMPTY(album,       vlc_meta_Get(m, vlc_meta_Album));
                 ON_EMPTY(genre,       vlc_meta_Get(m, vlc_meta_Genre));
                 ON_EMPTY(description, vlc_meta_Get(m, vlc_meta_Description));
+
+                author = vlc_meta_GetExtra(m, "AUTHOR");
+                composer = vlc_meta_GetExtra(m, "COMPOSER");
+                arranger = vlc_meta_GetExtra(m, "ARRANGER");
+                isrc = vlc_meta_GetExtra(m, "ISRC");
             }
         }
 
@@ -832,6 +843,15 @@ static int ReadDir(stream_t *access, input_item_node_t *node)
         if (NONEMPTY(album))
             input_item_SetAlbum(item, album);
 
+        if (NONEMPTY(author))
+            vlc_meta_AddExtra(item->p_meta, "AUTHOR", author);
+        if (NONEMPTY(composer))
+            vlc_meta_AddExtra(item->p_meta, "COMPOSER", composer);
+        if (NONEMPTY(arranger))
+            vlc_meta_AddExtra(item->p_meta, "ARRANGER", arranger);
+        if (NONEMPTY(isrc))
+            vlc_meta_AddExtra(item->p_meta, "ISRC", isrc);
+
         if (year != 0)
         {
             char yearbuf[5];


=====================================
modules/access/vcd/cdrom.c
=====================================
@@ -49,6 +49,7 @@
 #ifdef HAVE_ARPA_INET_H
 #include <arpa/inet.h>
 #endif
+#include <assert.h>
 
 #include <vlc_common.h>
 #include <vlc_access.h>
@@ -1194,6 +1195,24 @@ enum cdtext_charset_e
     CDTEXT_CHARSET_MSJIS = 0x80,
 };
 
+typedef enum {
+    cd_text_title       = 0x80,
+    cd_text_performer   = 0x81,
+    cd_text_songwriter  = 0x82,
+    cd_text_composer    = 0x83,
+    cd_text_arrangers   = 0x84,
+    cd_text_message     = 0x85,
+    cd_text_discid      = 0x86, // text & binary (track 0)
+    cd_text_genre       = 0x87, // text & binary (track 0)
+    cd_text_TOC         = 0x88, // binary (track 0)
+    cd_text_TOC2        = 0x89, // binary (track 0)
+    cd_text_closed_info = 0x8d, // (track 0)
+    cd_text_ean_isrc    = 0x8e,
+    cd_text_block_size  = 0x8f, // binary
+
+    cd_text_meta_invalid = 0x00,
+} cd_text_pack_type;
+
 static void CdTextAppendPayload( const char *buffer, size_t i_len,
                                  enum cdtext_charset_e e_charset, char **ppsz_text )
 {
@@ -1342,7 +1361,7 @@ static int CdTextParse( vlc_meta_t ***ppp_tracks, int *pi_tracks,
 {
     char *pppsz_info[CDTEXT_MAX_TRACKS + 1][0x10];
     int i_track_last = -1;
-    if( i_buffer < 4 )
+    if( i_buffer <= 4 )
         return -1;
 
     p_buffer += 4;
@@ -1354,9 +1373,10 @@ static int CdTextParse( vlc_meta_t ***ppp_tracks, int *pi_tracks,
     {
         const uint8_t *p_pack = &p_buffer[CDTEXT_PACK_SIZE*i];
         const uint8_t i_block_number = (p_pack[3] >> 4) & 0x07;
+        const cd_text_pack_type i_pack_type = p_pack[0];
         if( i_block_number > 0 )
             continue;
-        if( p_pack[0] == 0x8f )
+        if( i_pack_type == cd_text_block_size )
         {
             const int i_track = p_pack[1] & 0x7f;
             /* can't be higher than 3 blocks */
@@ -1390,12 +1410,12 @@ static int CdTextParse( vlc_meta_t ***ppp_tracks, int *pi_tracks,
     char textbuffer[CDTEXT_TEXT_BUFFER];
     size_t i_textbuffer = 0;
     size_t i_repeatbuffer = 0;
-    uint8_t i_prev_pack_type = 0x00;
+    cd_text_pack_type i_prev_pack_type = cd_text_meta_invalid;
 
     for( int i = 0; i < i_buffer/CDTEXT_PACK_SIZE; i++ )
     {
         const uint8_t *p_pack = &p_buffer[CDTEXT_PACK_SIZE*i];
-        const uint8_t i_pack_type = p_pack[0];
+        const cd_text_pack_type i_pack_type = p_pack[0];
         //const int i_sequence_number = p_block[2];
         const uint8_t i_block_number = (p_pack[3] >> 4) & 0x07;
         //const int i_crc = (p_block[4+12] << 8) | (p_block[4+13] << 0);
@@ -1408,35 +1428,33 @@ static int CdTextParse( vlc_meta_t ***ppp_tracks, int *pi_tracks,
         }
         i_prev_pack_type = i_pack_type;
 
-        uint8_t i_track = p_pack[1] & 0x7f;
-        if( i_track > CDTEXT_MAX_TRACKS ||
-            (p_pack[1] & 0x80) /* extension flag */ ||
+        if( (p_pack[1] & 0x80) /* extension flag */ ||
             i_block_number > 0 /* support only first language */
            )
         {
-            i_prev_pack_type = 0x00;
+            i_prev_pack_type = cd_text_meta_invalid;
             continue;
         }
 
         /* */
         switch( i_pack_type )
         {
-            case 0x80:
-            case 0x81:
-            case 0x85:
-            case 0x87:
+            case cd_text_title:
+            case cd_text_performer:
+            case cd_text_message:
+            case cd_text_genre:
+            case cd_text_songwriter:
+            case cd_text_composer:
+            case cd_text_arrangers:
+            case cd_text_ean_isrc:
             {
                 CdTextParsePackText( p_pack, e_textpackcharset,
                                      &i_textbuffer, &i_repeatbuffer, textbuffer,
                                      &i_track_last, pppsz_info );
                 break;
             }
-            case 0x82:
-            case 0x83:
-            case 0x84:
-            case 0x86:
-            case 0x8d:
-            case 0x8e:
+            case cd_text_discid:
+            case cd_text_closed_info:
             default:
                 continue;
         }
@@ -1456,6 +1474,11 @@ static int CdTextParse( vlc_meta_t ***ppp_tracks, int *pi_tracks,
             /* */
             const char *psz_default = pppsz_info[0][j];
             const char *psz_value = pppsz_info[i][j];
+            // discard junk values
+            if (psz_value && (psz_value[0] == '\0' || (psz_value[0] == ' ' && psz_value[1] == '\0')))
+                psz_value = NULL;
+            if (psz_default && (psz_default[0] == '\0' || (psz_default[0] == ' ' && psz_default[1] == '\0')))
+                psz_default = NULL;
 
             if( !psz_value && !psz_default )
                 continue;
@@ -1468,7 +1491,7 @@ static int CdTextParse( vlc_meta_t ***ppp_tracks, int *pi_tracks,
             }
             switch( 0x80 + j )
             {
-            case 0x80: /* Album/Title */
+            case cd_text_title:
                 if( i == 0 )
                 {
                     vlc_meta_SetAlbum( p_track, psz_value );
@@ -1481,23 +1504,43 @@ static int CdTextParse( vlc_meta_t ***ppp_tracks, int *pi_tracks,
                         vlc_meta_SetAlbum( p_track, psz_default );
                 }
                 break;
-            case 0x81: /* Performer */
+            case cd_text_performer:
                 vlc_meta_SetArtist( p_track,
                                     psz_value ? psz_value : psz_default );
+                if ( psz_value && i != 0 )
+                    vlc_meta_SetAlbumArtist( p_track, psz_default );
                 break;
-            case 0x85: /* Messages */
+            case cd_text_message:
                 vlc_meta_SetDescription( p_track,
                                          psz_value ? psz_value : psz_default );
                 break;
-            case 0x87: /* Genre */
+            case cd_text_genre:
                 vlc_meta_SetGenre( p_track,
                                    psz_value ? psz_value : psz_default );
                 break;
+            case cd_text_songwriter:
+                // lyrics
+                vlc_meta_AddExtra( p_track, "AUTHOR",
+                                   psz_value ? psz_value : psz_default );
+                break;
+            case cd_text_composer:
+                // music
+                vlc_meta_AddExtra( p_track, "COMPOSER",
+                                   psz_value ? psz_value : psz_default );
+                break;
+            case cd_text_arrangers:
+                vlc_meta_AddExtra( p_track, "ARRANGER",
+                                   psz_value ? psz_value : psz_default );
+                break;
+            case cd_text_ean_isrc:
+            {
+                if ( i == 0 )
+                    vlc_meta_AddExtra( p_track, "EAN/UPN", psz_default );
+                else if ( psz_value )
+                    vlc_meta_AddExtra( p_track, "ISRC", psz_value );
+            }
             /* FIXME unsupported:
-             * 0x82: songwriter
-             * 0x83: composer
-             * 0x84: arrenger
-             * 0x86: disc id */
+             * cd_text_discid */
             }
         }
     }
@@ -1531,15 +1574,13 @@ static int CdTextRead( vlc_object_t *p_object, const vcddev_t *p_vcddev,
 {
     VLC_UNUSED( p_object );
 
-    CDROM_READ_TOC_EX TOCEx;
-    memset(&TOCEx, 0, sizeof(TOCEx));
-    TOCEx.Format = CDROM_READ_TOC_EX_FORMAT_CDTEXT;
+    CDROM_READ_TOC_EX TOCEx = { .Format = CDROM_READ_TOC_EX_FORMAT_CDTEXT };
 
-    const int i_header_size = __MAX( 4, MINIMUM_CDROM_READ_TOC_EX_SIZE );
-    uint8_t header[i_header_size];
+    uint8_t header[4];
+    static_assert(ARRAY_SIZE(header) >= MINIMUM_CDROM_READ_TOC_EX_SIZE, "wrong header size");
     DWORD i_read;
     if( !DeviceIoControl( p_vcddev->h_device_handle, IOCTL_CDROM_READ_TOC_EX,
-                          &TOCEx, sizeof(TOCEx), header, i_header_size, &i_read, 0 ) )
+                          &TOCEx, sizeof(TOCEx), header, ARRAY_SIZE(header), &i_read, 0 ) )
         return -1;
 
     const int i_text = 2 + (header[0] << 8) + header[1];



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/e051b7d9bd87ba5783dcc22886560b08e78b97e8...84dbe80703a027658998fde5c8422586aa6bb5c0

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/e051b7d9bd87ba5783dcc22886560b08e78b97e8...84dbe80703a027658998fde5c8422586aa6bb5c0
You're receiving this email because of your account on code.videolan.org.




More information about the vlc-commits mailing list