Commit Graph

271 Commits

Author SHA1 Message Date
Peter Taylor
4fb9d99ddd Add missing 'Album Artist' tag when its value is 'Various Artists'
Fixes #518.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-12-25 14:30:00 +00:00
JoeLametta
c229c01a58 Replace 'freedb.dbpoweramp.com' CDDB server with gnudb.org
It seems gnudb.org allows submissions too while 'freedb.dbpoweramp.com' is read only.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-11-28 17:44:21 +00:00
JoeLametta
3e61c3dc1b Fix 'list index out of range' bug
It was introduced in commit acf942b5b6.

The error happens because 'self.itable.tracks' is a list of tracks
(zero-based numbering, HTOA excluded) so the first track can be accessed with
'self.itable.tracks[0]' but when 'number' has value 0, that matches the HTOA
(everything is shifted by one compared to 'self.itable.tracks').

It's unrelated to this bugfix but I've also moved some instructions outside
the try ... except clause.

Fixes #512.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-11-19 15:36:48 +00:00
JoeLametta
04ff005806 Fix bug in cdrom drive status handling
The bare if evaluated to true for return codes > 0 and that's wrong (CDS_DISC_OK = 4).

Fixes #511.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-10-12 12:20:53 +00:00
Matthew Peveler
a8942a8037 Update test_result_logger.log
Signed-off-by: Matthew Peveler <matt.peveler@gmail.com>
2020-09-24 03:58:32 -04:00
Matthew Peveler
1015d6e000 Fix capitalization of "Health status" in rip log
This fixes a regression from move to ruamel.yaml where all other fields in rip log have second word lowercased, except for "Health Status". This changes it to make it inline with all other fields again.

Signed-off-by: Matthew Peveler <matt.peveler@gmail.com>
2020-09-24 03:57:45 -04:00
JoeLametta
3c9e75c3bc Merge pull request #509 from whipper-team/feature/issue-320-isrc-audio-tag
Tag audio tracks with ISRCs (if available)
2020-09-23 19:56:54 +02:00
JoeLametta
84cc824f2e Merge pull request #505 from whipper-team/feature/issue-488-drive-auto-close
Allow configuring whether to auto close the drive's tray
2020-09-23 19:54:59 +02:00
JoeLametta
21185f40a9 Merge pull request #507 from whipper-team/bugfix/issue-385-missing-cd-warning
Provide better error message when there's no CD in the drive
2020-09-23 19:52:50 +02:00
JoeLametta
302bc22a66 Merge pull request #506 from whipper-team/ux/issue-495-cdparanoia-warnings
Add checks and warnings for (known) cdparanoia's upstream bugs
2020-09-23 19:51:16 +02:00
JoeLametta
8a1c0fabfc Allow configuring whether to auto close the drive's tray
Fixes #488.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-09-23 17:41:48 +00:00
JoeLametta
b754b2b0bf Restore getRipResult method to fix regression
The regression was introduced in commit 3acc3ffed6.
The getRipResult method has been slimmed down to its essence.

Fixes #508.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-09-20 13:07:14 +00:00
JoeLametta
acf942b5b6 Tag audio tracks with ISRCs (if available)
Fixes #320.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-09-19 20:14:22 +00:00
JoeLametta
1661e4291e Emit warning when the subcode's pre-emphasis flag of a track differs from the ToC's one
In the future I'd like to make sure this information is included in the logfile too (maybe also in the cue sheet).

Related to issue #296.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-09-18 11:58:18 +00:00
JoeLametta
921e25bf98 Provide better error message when there's no CD in the drive
Fixes #385.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-09-17 19:18:27 +00:00
JoeLametta
07bd0348f3 Add checks and warnings for (known) cdparanoia's upstream bugs
Fixes #495.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-09-17 18:07:28 +00:00
JoeLametta
3acc3ffed6 Drop whipper caching (#336)
Whipper's caching implementation causes a few issues (#196, #230, [#321 (comment)](https://github.com/whipper-team/whipper/pull/321#issuecomment-437588821)) and complicates the code: it's better to drop this feature.

The rip resume feature doesn't work anymore: if possible it will be restored in the future.

* Remove caching item from TODO
* Delete unneeded files related to caching
* Update 'common/directory.py' & 'test/test_common_directory.py' (caching removal)
* Update 'common/accurip.py' & 'test/test_common_accurip.py' (caching removal)
* Update 'common/program.py' (caching removal)
* Update 'command/cd.py' (caching removal)

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

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-09-17 17:52:11 +02:00
JoeLametta
bbed92bb30 Update failing AccurateRipResponse tests
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-08-10 16:39:55 +02:00
JoeLametta
fa7c50d3a6 Replace 'sys.exit()' and 'exit()' instructions with 'SystemExit()' equivalents
- `SystemExit` doesn't require importing the `sys` module
- `exit()` depends on the `site` module (for this reason its usage is discouraged in production code)

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-05-30 09:38:52 +00:00
JoeLametta
8802c5482e Improve error message for unconfigured drive offset
Fixes #478.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-05-29 16:24:30 +00:00
JoeLametta
752162a434 Fix two flake8 errors
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-05-29 14:03:27 +00:00
JoeLametta
bbf1eba0e4 Replace 'freedb.freedb.org' CDDB server with a mirror
This is motivated by the imminent shut down of freedb.org which will happen on 2020-03-31.

More details here: https://web.archive.org/web/20200331093822/http://www.freedb.org/
And here: https://hydrogenaud.io/index.php?topic=118682

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-03-31 13:22:21 +00:00
JoeLametta
4dc02ec12e Rewrite PathFilter
Added filter options:
- dot (replace leading dot with _)
- posix (replace illegal chars in *nix OSes with _)
- vfat (replace illegal chars in VFAT filesystems with _)
- whitespace (replace all whitespace chars with _)
- printable (replace all non printable ASCII chars with _)

Removed filter options:
- fat (replaced with vfat)
- special

Fixes #313.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-02-22 18:01:20 +00:00
Andreas Oberritter
afc31f930e config: generalize getting and setting of drive options
Fixed merge conflicts (JoeLametta).

Signed-off-by: Andreas Oberritter <obi@saftware.de>
2020-02-22 16:15:25 +00:00
JoeLametta
e56c636fd3 Improve docstrings
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-02-22 15:11:22 +00:00
JoeLametta
3b269e7a3b Fix flake8 warning
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-02-08 18:32:12 +00:00
JoeLametta
9a0b911666 Clarify 'set_hostname' warning message
Fixes #464.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-02-08 18:24:00 +00:00
JoeLametta
6fd7a78222 Change dest name of '--cover-art' option to 'cover_art'
Make it consistent with the name of the option.

Fixes #465.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-02-08 17:51:29 +00:00
JoeLametta
9e63915f65 Display release country in matching releases
This simplifies choosing the correct release when there are multiple matches.
If a certain release has multiple countries associated, all will be shown.

Thanks to the user "the-confessor" for testing this new feature.

Fixes #451.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-02-04 16:36:02 +01:00
JoeLametta
3213241ea5 Merge pull request #461 from neilmayhew/fix/inline-config-comments
Restore the ability to use inline comments in config files
2020-02-04 16:23:55 +01:00
Neil Mayhew
dca9fcb7dc Restore the ability to use inline comments in config files
The ability was lost in the switch to Python 3, because the config
parser module in the standard library changed its defaults.

[Python 2][2]:

> Comments may appear on their own in an otherwise empty line, or
> may be entered in lines holding values or section names.

[Python 3][3]:

> Inline comments can be harmful because they prevent users
> from using the delimiting characters as parts of values.
> That being said, this can be customized.

[2]: https://docs.python.org/2/library/configparser.html#module-ConfigParser
[3]: https://docs.python.org/3/library/configparser.html#supported-ini-file-structure

Signed-off-by: Neil Mayhew <neil@neil.mayhew.name>
2020-02-03 15:01:36 -07:00
JoeLametta
87e75d0f98 Drop 'requests' external dependency
It was only used in a single method and wasn't really needed.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-02-03 15:56:07 +00:00
Kevin Locke
922389687a Fix cd rip --max-retries option handling
9db3aa9 introduced the -r/--max-retries option, but passed `'r'` to
`argparse.ArgumentParser.add_argument` instead of `'-r'` which causes:

    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File ".../whipper/command/main.py", line 48, in main
        cmd = Whipper(sys.argv[1:], os.path.basename(sys.argv[0]), None)
      File ".../whipper/command/basecommand.py", line 117, in __init__
        self.options
      File ".../whipper/command/basecommand.py", line 117, in __init__
        self.options
      File ".../whipper/command/basecommand.py", line 60, in __init__
        self.add_arguments()
      File ".../whipper/command/cd.py", line 308, in add_arguments
        default=DEFAULT_MAX_RETRIES)
      File "/usr/lib/python3.7/argparse.py", line 1354, in add_argument
        kwargs = self._get_optional_kwargs(*args, **kwargs)
      File "/usr/lib/python3.7/argparse.py", line 1485, in _get_optional_kwargs
        raise ValueError(msg % args)
    ValueError: invalid option string 'r': must start with a character '-'

for any arguments passed to `whipper cd rip`.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
2020-01-29 16:40:24 -07:00
JoeLametta
1a06e51c80 Add whipper useragent to AccurateRip requests
Don't think it's required but it would be impolite not to announce
the software making the requests with its name, version and
contact information.

Fixes #439.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-01-29 14:58:09 +00:00
JoeLametta
dea7db10cf Merge pull request #459 from kevinoid/fix-unknown-no-mbid
Fix crash fetching cover art for unknown album
2020-01-29 13:42:44 +01:00
Kevin Locke
3ec17dbd33 Fix crash fetching cover art for unknown album
Ripping an unknown album when cover art fetching is enabled (e.g.
`whipper cd rip --unknown --cover-art complete`) causes whipper to crash
with an error similar to the following:

```python
Traceback (most recent call last):
    File "<string>", line 1, in <module>
    File ".../whipper/whipper/command/main.py", line 43, in main
    ret = cmd.do()
    File ".../whipper/whipper/command/basecommand.py", line 139, in do
    return self.cmd.do()
    File ".../whipper/whipper/command/basecommand.py", line 139, in do
    return self.cmd.do()
    File ".../whipper/whipper/command/cd.py", line 191, in do
    self.doCommand()
    File ".../whipper/whipper/command/cd.py", line 363, in doCommand
    self.program.metadata.mbid)
AttributeError: 'NoneType' object has no attribute 'mbid'
```

due to accessing `self.program.metadata.mbid` when
`self.program.metadata` is `None`. To avoid this, only attempt to get
cover art when `self.program.metadata` is available.

Also print a warning when the cover art can't be fetched to inform the
user that it isn't being downloaded.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
2020-01-29 12:35:49 +00:00
JoeLametta
eaf96ea64e Merge pull request #455 from ABCbum/customize-max-tries
Allow customization of maximum rip attempts value
2020-01-29 12:42:04 +01:00
ABCbum
9db3aa9247 Allow customization of maximum rip attempts value
Add new `--max-retries` argument to allow users to specify maximum number
of attempts to try before giving up ripping a track. This value defaults to `5` while `0` means infinity.
Possible errors (negative number, string, etc) are also handled.

Co-authored-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: ABCbum <kimlong221002@gmail.com>
2020-01-29 11:36:55 +00:00
JoeLametta
087a53a7d0 Merge pull request #444 from ABCbum/add-alternative-tagging
Add PERFORMER & COMPOSER metadata tags to audio tracks (if available)
2020-01-29 10:01:27 +01:00
JoeLametta
7b8a20b22b Merge pull request #450 from ABCbum/develop
Use https and http appropriately when connecting to MusicBrainz
2020-01-29 10:00:35 +01:00
JoeLametta
56f2b1d5f5 Merge pull request #456 from ABCbum/add-version-test
Test all four cases of whipper version scheme
2020-01-29 09:51:43 +01:00
ABCbum
f59caeae7b Test all four cases of whipper version schemes
Group different version schemes with the actual one generated from the logger in a list to avoid parsing a whole .log file.
The four possible cases are documented here: https://github.com/pypa/setuptools_scm/#default-versioning-scheme.

Fixes: #427.

Signed-off-by: ABCbum <kimlong221002@gmail.com>
2020-01-29 08:45:07 +00:00
JoeLametta
b39345e1f0 Use shutil.move() instead of os.replace/rename
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-01-28 21:50:27 +00:00
JoeLametta
7f438d1b75 Improve help string consistency
Reported by user "ABCbum" in comment (https://github.com/whipper-team/whipper/pull/436#discussion_r370068256).

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-01-28 17:05:31 +00:00
Kevin Locke
b6607c6573 Fix cover file saving with /tmp on different FS
If the directory used by tempfile.NamedTemporaryFile is on a different
filesystem (e.g. /tmp on tmpfs), `whipper cd rip --cover-art` will fail
with an error such as:

    Traceback (most recent call last):
      File "/usr/bin/whipper", line 11, in <module>
        load_entry_point('whipper==0.9.0', 'console_scripts', 'whipper')()
      File "/home/kevin/tmp/whipper/whipper/command/main.py", line 43, in main
        ret = cmd.do()
      File "/home/kevin/tmp/whipper/whipper/command/basecommand.py", line 139, in do
        return self.cmd.do()
      File "/home/kevin/tmp/whipper/whipper/command/basecommand.py", line 139, in do
        return self.cmd.do()
      File "/home/kevin/tmp/whipper/whipper/command/cd.py", line 191, in do
        self.doCommand()
      File "/home/kevin/tmp/whipper/whipper/command/cd.py", line 363, in doCommand
        self.program.metadata.mbid)
      File "/home/kevin/tmp/whipper/whipper/common/program.py", line 498, in getCoverArt
        os.replace(f.name, cover_art_path)
    OSError: [Errno 18] Invalid cross-device link: '/tmp/tmprmx4d9c9.cover.jpg' -> './Boston/Greatest Hits/cover.jpg'

due to calling os.replace with paths on different filesystems.

Instead of os.replace, use shutil.move, which falls back to shutil.copy2
if os.replace doesn't work.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
2020-01-26 17:37:11 -07:00
ABCbum
0daf0158b8 Test: verify that MusicBrainz lookup URL defaults to https
Applies to a default configuration (no custom MusicBrainz server specified).

Co-authored-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: ABCbum <kimlong221002@gmail.com>
2020-01-17 15:23:11 +00:00
JoeLametta
1206552bd2 Use https and http appropriately when connecting to MusicBrainz
Fixed some bugs:
- MusicBrainz submit URL always has https as protocol: hardcoded, even when
inappropriate. It's just a graphical issue.
- Whipper appears to always communicate with MusicBrainz using musicbrainzngs
over http. The musicbrainzngs.set_hostname(server).
- `musicbrainzngs.set_hostname(server)` always defaults to http. Since musicbrainzngs
version 0.7 the method `set_hostname` takes an optional argument named `use_https`
(defaults to False) which whipper never passes.

Changed behaviour of `server` option (`musicbrainz` section of whipper's configuration file).
Now it expects an URL with a valid scheme (scheme must be `http` or `http`, empty scheme isn't allowed anymore).
Only the scheme and netloc parts of the URL are taken into account.

Fixes #437.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-01-17 15:12:40 +00:00
ABCbum
b79236ee11 Add test to check _getPerformers and _getComposers
Signed-off-by: ABCbum <kimlong221002@gmail.com>
Co-authored-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-01-15 13:16:16 +07:00
ABCbum
5cd96da6cb Extend whipper's tagging ability
Add PERFORMER & COMPOSER metadata tags to audio tracks (if available).
Composer(s) and performer(s) will be extracted from MusicBrainz
recording metadata by new _getComposers and _getPerformers
functions then there will be new properties added to each track
metadata. If those data are present it will be tagged as new tags
PERFORMER and COMPOSER.

Signed-off-by: ABCbum <kimlong221002@gmail.com>
Co-authored-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-01-15 13:13:53 +07:00
JoeLametta
caa2c8b27d Bug: whipper shouldn't abort if track rip succeeds on last allowed retry attempt
Fixes #449.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-01-14 17:50:04 +00:00