Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop whipper caching #336

Merged
merged 6 commits into from
Sep 17, 2020
Merged

Drop whipper caching #336

merged 6 commits into from
Sep 17, 2020

Conversation

JoeLametta
Copy link
Collaborator

@JoeLametta JoeLametta commented Nov 21, 2018

Whipper's caching implementation causes a few issues (#196, #230, #321 (comment)) and complicates the code: it's better to drop this feature.

This fixes #335, fixes #196 and fixes #230.

Copy link
Member

@Freso Freso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love how much code this gets rid of. Seems like it will make the code simpler indeed. One small wording (docstring) change and please also just get rid of the cache_path() stuff. It's not used anywhere anymore according to git grep.

I haven't tested this though, just reviewed it "by eye", so it may well break everything. 😂 I'll try and find some time to run a rip using this branch.

whipper/common/accurip.py Outdated Show resolved Hide resolved
whipper/common/directory.py Outdated Show resolved Hide resolved
whipper/common/program.py Outdated Show resolved Hide resolved
whipper/test/test_common_directory.py Outdated Show resolved Hide resolved
@JoeLametta
Copy link
Collaborator Author

Regarding to the rip resume thing I think it may work without caching too but the logfile is going to miss some information about the tracks which have been already ripped.

  1:
    Filename: /path/to/track.flac
    Pre-gap length: 00:00:50
    Peak level: 0.1234567
    Pre-emphasis: No
    Extraction speed: 1.2 X
    Extraction quality: 100.00 %
    Test CRC: 0CFA7ABF
    Copy CRC: 0CFA7ABF
    AccurateRip v1:
        Result: Found, exact match
        Confidence: 9
        Local CRC: CD2E3B53
        Remote CRC: CD2E3B53
    AccurateRip v2:
        Result: Found, exact match
        Confidence: 19
        Local CRC: 3D05ABCF
        Remote CRC: 3D05ABCF
    Status: Copy OK

@JoeLametta JoeLametta force-pushed the bugfix/issue-335-drop-caching branch 2 times, most recently from 18f971c to ab4e7f4 Compare December 14, 2018 21:28
@JoeLametta JoeLametta force-pushed the bugfix/issue-335-drop-caching branch 2 times, most recently from 1faa9de to 7009329 Compare February 2, 2019 20:15
@JoeLametta JoeLametta force-pushed the bugfix/issue-335-drop-caching branch 3 times, most recently from d109a43 to 68d4bb6 Compare November 7, 2019 10:54
@JoeLametta
Copy link
Collaborator Author

Rebased (fixed merge conflicts).

1 similar comment
@JoeLametta
Copy link
Collaborator Author

Rebased (fixed merge conflicts).

@JoeLametta
Copy link
Collaborator Author

Fixed merge conflict.

@JoeLametta
Copy link
Collaborator Author

Fixed merge conflicts.

@JoeLametta
Copy link
Collaborator Author

Shall we revive this one?

@jwflory
Copy link

jwflory commented Apr 16, 2020

It would be cool to see this get merged. 😄

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
…ng removal)

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
…emoval)

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta JoeLametta changed the title WIP: Drop whipper caching Drop whipper caching Sep 17, 2020
@JoeLametta JoeLametta merged commit 3acc3ff into develop Sep 17, 2020
@JoeLametta JoeLametta deleted the bugfix/issue-335-drop-caching branch September 17, 2020 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants