benfmiller / audalign

Package for aligning audio files through audio fingerprinting
MIT License
84 stars 2 forks source link

finally of _allign wipes important data #23

Closed pursuk closed 2 years ago

pursuk commented 2 years ago

Hi again, this time its lines 114-117 in align.py

    finally:
        ada_obj.file_names = temp_file_names
        ada_obj.fingerprinted_files = temp_fingerprinted_files
        ada_obj.total_fingerprints = temp_total_fingerprints

all the temps seem to be empty when declared and they never appear again in the code. I noticed because save_fingerprinted_files was saving 3 empty variables that get replaced with empty temp_ files in this snippet. Removing these 3 lines solves the issue but probably isnt the intentional behavior nor an optimal solution.

benfmiller commented 2 years ago

Hello,

I wasn't quite sure how to handle the previously fingerprinted files. The try at the start saves the state of the fingerprints then the finally puts them back after aligning is done. That way you could do some fingerprints and recognitions before and after aligning without interrupting prior fingerprinting. I recognize that's not a way many people would use this program, though.

Doing it like you did is an option and might be a little niftier than it is now, but clears any prior fingerprints. But it's totally an option.

I could include an option in the align functions to load a saved fingerprints file. That way you could fingerprint files, save them to a file, then do alignments with that fingerprints file. The saved fingerprints don't save any details about settings, so it would be harder to record the settings to go along with each alignment.

pursuk commented 2 years ago

I am not sure i understood correctly. Considering current state of the code:

    ada.target_align(master, tracks)
    ada.save_fingerprinted_files('file.json')

is actually supposed to

because there is no mean of loading in previous fingerprints either way (for purpose of target_align or any align function, as it is intended to be a standalone process that does its own fingerprinting). Is that correct?

benfmiller commented 2 years ago

Yep! That is correct. That is how it currently works.

I'm thinking an argument to load from a saved fingerprints file in the align functions is the best option to implement. Do you have input on what you think might be a better option?

benfmiller commented 2 years ago

You can now load fingerprints inside of an alignment with the load_fingerprints option