crash in best_bitrate_file() processing a CSV playlist as if it has a bitrate

[skierpage]

I ran this and after a few seconds it died with

AttributeError: 'NoneType' object has no attribute 'info'

in best_bitrate_file() on

fl = [(mutagen.File(item.full_path).info.bitrate, item) for item in mfi_filelist]

because item.full_path contains /home/spage/Music/GPM_Playlists/80s UK/Tracks/Southern Freeez.csv.

This seems to be assuming that the items in a mfi_filelist are MP3s that mutagen can pull bitrate from, but obviously a CSV playlist file does not contain any bitrate info :wink: , so mutagen.File(item.full_path).info is None, let alone having a bitrate in it. Does the script assume that the user's playlists are not inside the MUSIC_PATH directory? I have all kinds of files in $HOME/Music besides digital music files. I could name the particular directories in there that only include digital music files, but os.walk(MUSIC_PATH) doesn't allow a set of paths.

At the point of the crash, mfi_file_list contains this CSV file twice in two different playlists: [FileInfo(full_path='/home/spage/Music/GPM_Playlists/80s UK/Tracks/Southern Freeez.csv', filename='Southern Freeez.csv', tag=None), FileInfo(full_path='/home/spage/Music/GPM_Playlists/Thumbs Up/Southern Freeez.csv', filename='Southern Freeez.csv', tag=None)]

[lucidBrot]

When I used that script myself, I definitely had other files in the MUSIC_PATH. Not .csv files, but for example images. Maybe I had no issues because best_bitrate_file will only be executed if there are multiple files that look like they could match the song.

I haven't had the time yet to look at it in more detail. But if the problem is just that one line in best_bitrate_file then perhaps replacing it with something along these lines will work for you:

def best_bitrate_file(mfi_filelist_unfiltered):
    # make sure there are no non-music files in there
    mfi_filelist = []
    for item in mfi_filelist_unfiltered:
        file_obj = mutagen.File(item.full_path)
        file_info = file_obj.info if ( file_obj is not None ) else None
        if file_info is not None and file_info.bitrate is not None:
            mfi_filelist.append( item )

    # choose the option with the best bitrate
    if len(mfi_filelist) < 1:
        return None
    fl = [(mutagen.File(item.full_path).info.bitrate, item) for item in mfi_filelist]
    sl = sorted(fl, reverse=True, key=lambda x:x[0])
    return sl[0][1]

I just typed this up in the browser textbox. Let me know if it works for you and if it does I'll fix the script in the repo here as well (and if it does not work, it'll be helpful to know more) :)

Thanks for reporting! 😄

[skierpage]

Thanks for being so responsive. I had worked around the problem by adding GPM_Playlists to IGNORE_MUSIC_FOLDERS. I reverted that local change and tried your fix to best_bitrate_file() and it got past that crash, but now crashes in filepath_contains_info() on

    print("... choosing the best bitrate file: {}".format(best_bitrate_f.full_path))
AttributeError: 'NoneType' object has no attribute 'full_path'

because the modified best_bitrate_file now returns None, because the mfi_file_list doesn't contain MP3s, just two the two .csvs from two playlists.

I really appreciate your attention to my attempts to use this script, but I think it's inappropriate for my use case. My playlists are mostly songs I've streamed from Google Play Music All Access, so even if I work around this problem, the step where the script complains that it can't find a bunch of songs from my playlists, and invites me to fix up _missing_matches.json, makes no sense. All I'm trying to do is make sure that I have local copies of the files that I uploaded to GPM with Music Manager and are available as MP3s in the takeout.zips.

[lucidBrot]

Hm, okay. I will have a more in-depth look at the crashes when life permits, because this is probably a bug even if your usecase is different.

All I'm trying to do is make sure that I have local copies of the files that I uploaded to GPM with Music Manager and are available as MP3s in the takeout.zips.

That is indeed not what I used the script for. Thanks for all the info though, it might be useful to others :blush: