[vlc-devel] [PATCH] Do not use album art if the file has a built-in cover.
Thomas Guillem
thomas at gllm.fr
Tue Jan 26 10:53:33 UTC 2021
Hello,
Sorry for the very long delay.
On Tue, Jan 12, 2021, at 19:17, Samo Golež wrote:
> Currently, VLC uses the same image if the files are from the same
> album, but with this PATCH, if the file has a built-in image, it will
> be used instead of the album image.
> ---
> src/input/meta.c | 2 ++
> src/preparser/art.c | 54 ++++++++++++++++++++++++++++++++++-------
> src/preparser/art.h | 1 +
> src/preparser/fetcher.c | 38 +++++++++++++++++++++--------
> src/preparser/fetcher.h | 3 +++
> 5 files changed, 79 insertions(+), 19 deletions(-)
>
> diff --git a/src/input/meta.c b/src/input/meta.c
> index 9ef43ca..ba7a97b 100644
> --- a/src/input/meta.c
> +++ b/src/input/meta.c
> @@ -212,6 +212,8 @@ void input_ExtractAttachmentAndCacheArt(
> input_thread_t *p_input,
> msg_Warn( p_input, "art already fetched" );
> if( likely(input_FindArtInCache( p_item ) == VLC_SUCCESS) )
> return;
> + if( likely(input_FindAlbumArtInCache( p_item ) == VLC_SUCCESS)
> )
> + return;
> }
>
> /* */
> diff --git a/src/preparser/art.c b/src/preparser/art.c
> index 9fa9e66..2256b04 100644
> --- a/src/preparser/art.c
> +++ b/src/preparser/art.c
> @@ -56,12 +56,12 @@ static void ArtCacheCreateDir( char *psz_dir )
>
> static char* ArtCacheGetDirPath( const char *psz_arturl, const char
> *psz_artist,
> const char *psz_album, const char
> *psz_date,
> - const char *psz_title )
> + const char *psz_title, bool b_unique
> )
> {
> char *psz_dir;
> char *psz_cachedir = config_GetUserDir(VLC_CACHE_DIR);
>
> - if( !EMPTY_STR(psz_artist) && !EMPTY_STR(psz_album) )
> + if( !EMPTY_STR(psz_artist) && !EMPTY_STR(psz_album) && !b_unique )
> {
> char *psz_album_sanitized = strdup( psz_album );
> if (!psz_album_sanitized)
> @@ -108,6 +108,8 @@ static char* ArtCacheGetDirPath( const char
> *psz_arturl, const char *psz_artist,
>
> vlc_hash_md5_t md5;
> vlc_hash_md5_Init( &md5 );
> + if (!psz_arturl)
> + return NULL;
> vlc_hash_md5_Update( &md5, psz_arturl, strlen( psz_arturl ) );
> if( !strncmp( psz_arturl, "attachment://", 13 ) )
> vlc_hash_md5_Update( &md5, psz_title, strlen( psz_title )
> );
> @@ -120,7 +122,7 @@ static char* ArtCacheGetDirPath( const char
> *psz_arturl, const char *psz_artist,
> return psz_dir;
> }
>
> -static char *ArtCachePath( input_item_t *p_item )
> +static char *ArtCachePath( input_item_t *p_item, bool b_unique )
> {
> char* psz_path = NULL;
> const char *psz_artist;
> @@ -147,16 +149,16 @@ static char *ArtCachePath( input_item_t *p_item )
> if( (EMPTY_STR(psz_artist) || EMPTY_STR(psz_album) ) && !psz_arturl )
> goto end;
>
> - psz_path = ArtCacheGetDirPath( psz_arturl, psz_artist, psz_album,
> psz_date, psz_title );
> + psz_path = ArtCacheGetDirPath( psz_arturl, psz_artist, psz_album,
> psz_date, psz_title, b_unique );
>
> end:
> vlc_mutex_unlock( &p_item->lock );
> return psz_path;
> }
>
> -static char *ArtCacheName( input_item_t *p_item, const char *psz_type )
> +static char *ArtCacheName( input_item_t *p_item, const char *psz_type,
> bool b_unique )
> {
> - char *psz_path = ArtCachePath( p_item );
> + char *psz_path = ArtCachePath( p_item, b_unique );
> char *psz_ext = strdup( psz_type ? psz_type : "" );
> char *psz_filename = NULL;
>
> @@ -177,9 +179,9 @@ end:
> }
>
> /* */
> -int input_FindArtInCache( input_item_t *p_item )
> +static int input_FindInCache( input_item_t *p_item, bool b_unique )
> {
> - char *psz_path = ArtCachePath( p_item );
> + char *psz_path = ArtCachePath( p_item, b_unique );
>
> if( !psz_path )
> return VLC_EGENERIC;
> @@ -221,6 +223,18 @@ int input_FindArtInCache( input_item_t *p_item )
> return b_found ? VLC_SUCCESS : VLC_EGENERIC;
> }
>
> +/* Find find file art */
> +int input_FindArtInCache( input_item_t *p_item )
> +{
> + return input_FindInCache(p_item, true);
> +}
> +
> +/* Find album art */
> +int input_FindAlbumArtInCache( input_item_t *p_item )
> +{
> + return input_FindInCache(p_item, false);
> +}
> +
> static char * GetDirByItemUIDs( char *psz_uid )
> {
> char *psz_cachedir = config_GetUserDir(VLC_CACHE_DIR);
> @@ -286,7 +300,29 @@ int input_FindArtInCacheUsingItemUID( input_item_t
> *p_item )
> int input_SaveArt( vlc_object_t *obj, input_item_t *p_item,
> const void *data, size_t length, const char
> *psz_type )
> {
> - char *psz_filename = ArtCacheName( p_item, psz_type );
> + // save album
> + char *psz_album_filename = ArtCacheName( p_item, psz_type, false );
So, will this result on saving embedded covers for every file belonging to one album ?
These covers will be likely the same, and we have already seen users having HUGE embedded cover.
> +
> + struct stat s_album;
> + if( psz_album_filename && vlc_stat( psz_album_filename, &s_album )
> )
> + {
> + FILE *f = vlc_fopen( psz_album_filename, "wb" );
> + if( f )
> + {
> + if( fwrite( data, 1, length, f ) != length )
> + {
> + msg_Err( obj, "%s: %s", psz_album_filename,
> vlc_strerror_c(errno) );
> + }
> + else
> + {
> + msg_Dbg( obj, "album art saved to %s",
> psz_album_filename );
> + }
> + fclose( f );
> + }
Could you refactor this part? Add a new function that check if the file exist and write the data.
> + }
> +
> + // now save unique
> + char *psz_filename = ArtCacheName( p_item, psz_type, true );
>
> if( !psz_filename )
> return VLC_EGENERIC;
> diff --git a/src/preparser/art.h b/src/preparser/art.h
> index e4cb82f..8bb0cc2 100644
> --- a/src/preparser/art.h
> +++ b/src/preparser/art.h
> @@ -25,6 +25,7 @@
> #define _INPUT_ART_H 1
>
> int input_FindArtInCache( input_item_t * );
> +int input_FindAlbumArtInCache( input_item_t * );
> int input_FindArtInCacheUsingItemUID( input_item_t * );
>
> int input_SaveArt( vlc_object_t *, input_item_t *,
> diff --git a/src/preparser/fetcher.c b/src/preparser/fetcher.c
> index 7616078..88a8911 100644
> --- a/src/preparser/fetcher.c
> +++ b/src/preparser/fetcher.c
> @@ -144,7 +144,7 @@ Submit(input_fetcher_t *fetcher, vlc_executor_t
> *executor, input_item_t *item,
> return VLC_SUCCESS;
> }
>
> -static char* CreateCacheKey( input_item_t* item )
> +static char* CreateCacheKey( input_item_t* item , bool b_album )
> {
> vlc_mutex_lock( &item->lock );
>
> @@ -169,6 +169,19 @@ static char* CreateCacheKey( input_item_t* item )
> {
> key = NULL;
> }
> +
> + if (key && !b_album)
> + {
> + char const *arturl = vlc_meta_Get( item->p_meta, vlc_meta_ArtworkURL );
> + char const *title = vlc_meta_Get( item->p_meta, vlc_meta_Title );
Artist + Title is not enough? Why are you using ArtworkURL as a cache key ?
> +
> + if( !arturl || !title || asprintf( &key, "%s:%zu:%s:%zu",
> + arturl, strlen( arturl ), title, strlen( title ) ) < 0 )
> + {
> + key = NULL;
> + }
> + }
> +
> vlc_mutex_unlock( &item->lock );
>
> return key;
> @@ -180,9 +193,9 @@ static void FreeCacheEntry( void* data, void* obj )
> VLC_UNUSED( obj );
> }
>
> -static int ReadAlbumCache( input_fetcher_t* fetcher, input_item_t*
> item )
> +static int ReadCache( input_fetcher_t* fetcher, input_item_t* item,
> bool b_album )
> {
> - char* key = CreateCacheKey( item );
> + char* key = CreateCacheKey( item, b_album );
>
> if( key == NULL )
> return VLC_EGENERIC;
> @@ -198,11 +211,11 @@ static int ReadAlbumCache( input_fetcher_t*
> fetcher, input_item_t* item )
> return art ? VLC_SUCCESS : VLC_EGENERIC;
> }
>
> -static void AddAlbumCache( input_fetcher_t* fetcher, input_item_t* item,
> - bool overwrite )
> +static void AddCache( input_fetcher_t* fetcher, input_item_t* item,
> + bool b_album, bool overwrite )
> {
> char* art = input_item_GetArtURL( item );
> - char* key = CreateCacheKey( item );
> + char* key = CreateCacheKey( item, b_album );
>
> if( key && art && strncasecmp( art, "attachment://", 13 ) )
> {
> @@ -278,12 +291,15 @@ static int SearchByScope(struct task *task, int scope)
> }
>
> if( ! CheckArt( item ) ||
> - ! ReadAlbumCache( fetcher, item ) ||
> + ! ReadCache( fetcher, item, ALBUM ) ||
> + ! ReadCache( fetcher, item, SONG ) ||
> ! input_FindArtInCacheUsingItemUID( item ) ||
> ! input_FindArtInCache( item ) ||
> + ! input_FindAlbumArtInCache( item ) ||
> ! SearchArt( fetcher, item, scope ) )
> {
> - AddAlbumCache( fetcher, task->item, false );
> + AddCache( fetcher, task->item, SONG, false );
> + AddCache( fetcher, task->item, ALBUM, false );
> int ret = Submit(fetcher, fetcher->executor_downloader, item,
> task->options, task->cbs, task->userdata);
> if (ret == VLC_SUCCESS)
> @@ -306,7 +322,8 @@ static void RunDownloader(void *userdata)
>
> vlc_interrupt_set(&task->interrupt);
>
> - ReadAlbumCache( fetcher, task->item );
> + if ( !!ReadCache( fetcher, task->item, SONG ) )
Compare directly with VLC_SUCCESS or VLC_EGENERIC please.
> + ReadCache( fetcher, task->item, ALBUM );
>
> char *psz_arturl = input_item_GetArtURL( task->item );
> if( !psz_arturl )
> @@ -351,7 +368,8 @@ static void RunDownloader(void *userdata)
> output_stream.length, NULL );
>
> free( output_stream.ptr );
> - AddAlbumCache( fetcher, task->item, true );
> + AddCache( fetcher, task->item, SONG, true );
> + AddCache( fetcher, task->item, ALBUM, true );
>
> out:
> vlc_interrupt_set(NULL);
> diff --git a/src/preparser/fetcher.h b/src/preparser/fetcher.h
> index 680386a..609f921 100644
> --- a/src/preparser/fetcher.h
> +++ b/src/preparser/fetcher.h
> @@ -26,6 +26,9 @@
>
> #include <vlc_input_item.h>
>
> +#define ALBUM true
> +#define SONG false
You should use an enum.
> +
> /**
> * Fetcher opaque structure.
> *
> --
> 2.27.0
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list