Commit Graph

247 Commits

Author SHA1 Message Date
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
ABCbum
e2942b07e3 Add test case to check getCoverArt's functionality
Mock two functions `getCoverArt`, `get_image_front` and use
a locally available cover art to check if the created cover
art exists.

Problems:
- How to check image's quality.
- Not sure if only this check is enough (do we need to check the
embedding part?).

Signed-off-by: ABCbum <kimlong221002@gmail.com>
2020-01-14 15:57:34 +00:00
ABCbum
f61214a238 Support fetching cover art images from the Cover Art Archive
Add option `--cover-art` to `whipper cd rip` command which accepts three values:
- `file`: save the downloaded cover image as standalone file in the rip folder (named `cover.jpg`)
- `embed`: embed the download cover image into all the ripped audio tracks (no standalone file will be kept)
- `complete`: save standalone cover image as standalone file and embed it into all the ripped audio tracks (`file` + `embed`)

Every cover art is fetched from the Cover Art Archive as JPEG thumbnail with a maximum dimension of 500px.
Other supported values for the thumbnails are 250, 500 and 1200 (currently only some images have a corresponding 1200px sized thumbnail).

This feature introduces an optional dependency on the `Pillow` module which is required for the decoding of the cover file (required by the `embed` and `complete` option values).

Problem:
- EmbedPicTureTask shouldn't be a task.

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-14 15:57:34 +00:00
JoeLametta
fb9fb34b83 Move comment to the right place
The blank line after the comment was added in commit 644e67f105.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2020-01-04 12:21:08 +00:00
ABCbum
97ffd0fe4d Add test case when data track is first track
Using existing TOCs, create a new test case to verify discid generated
when data track is not at the end of the disc track-list.

Quality of test is not verified.

Signed-off-by: ABCbum <kimlong221002@gmail.com>
2019-12-28 12:42:26 +00:00
ABCbum
a113404c33 Replace whipper's disc id calculation with discid
Since whipper's own "musicbrainz id calculation" fails on CDs with data
tracks on special places, the disc id calculation code is replaced with
libdiscid.

Gives a new way to calculate disc leadout or sectors (last sector of last
audio track) depends on whether the data track is placed last or not.

`discid` requires `len(track_offsets) != last - first + 1`.Which means
there can be only one data track in the disc and the lastTrack number is
deceptive.

For example: a disc (data audio audio audio) has firstTrack=1 lastTrack=3
which is wrong since according to the discid code :
    "last **audio** track as :obj:`int"
it should be 4 but the firstTrack is always 1.

The code is duplicated with _getMusicBrainzValues function.

Signed-off-by: ABCbum <kimlong221002@gmail.com>
2019-12-28 12:42:26 +00:00
JoeLametta
8f5559ebc8 Merge pull request #432 from ABCbum/develop
Allow whipper's mblookup command to look up information based on Release MBID
2019-12-19 10:01:10 +01:00
ABCbum
bb66a092cd Add test case to new mblookup functionality
A new test case to check for mblookup's ability to search
for data based on release id is created along with a function
mocks getReleaseMetadata using an existing JSON file.

Signed-off-by: ABCbum <kimlong221002@gmail.com>
2019-12-19 08:56:00 +00:00
ABCbum
78c91fd1c7 Add new functionality and refactor code
In order to make mblookup command able to lookup data based
on release id, new function getReleaseMetadata is created.
To remove duplicated code, importing and set_useragent is moved
to the top of the file.
Now _getMetadata behaves differently when the discid is not
specified.

Signed-off-by: ABCbum <kimlong221002@gmail.com>
2019-12-19 08:55:54 +00:00
ABCbum
b914b31119 Enable mblookup to take release id as argument
To make mblookup able to look up data based on release id,
RegExp is used to detect whether the input is release id
or disc id and behaves differently according to that.
The input is now also tripped before being passed down.

Signed-off-by: ABCbum <kimlong221002@gmail.com>
2019-12-19 08:55:13 +00:00
Merlijn Wajer
29ee670b7f program/cdparanoia: fix failed() task of AnalyzeTask
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2019-12-15 15:48:30 +00:00
ABCbum
31d589b00d Enable whipper to use track title (#430)
* Enable whipper to use track title if possible

track.title = t.get('title', t['recording']['title']).

Since if a track itself doesn't have a title then the track
title is the same with the recording title. Otherwise, a track
has its own title then t['title'] is different from
t['recording']['title'] and whipper chooses t['title'].

[Fixes #192]
Signed-off-by: ABCbum <kimlong221002@gmail.com>

* Add test case to check for track title

Using an existing JSON release file

Signed-off-by: ABCbum <kimlong221002@gmail.com>
2019-12-13 18:43:48 +01:00
JoeLametta
c13f541e61 Push whipper release v0.9.0
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2019-12-04 13:26:33 +00:00
JoeLametta
d5bf83e4cc Whipper's version RegEX: support all valid scheme cases
Initial work by ArchangeGabriel: https://github.com/whipper-team/whipper/pull/421

Fixes #420.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2019-12-03 10:53:05 +00:00
Merlijn Wajer
47c62a9990 Fix regression introduced in Python 3 port
Fixes #424.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2019-12-03 10:39:46 +00:00
JoeLametta
bb4c25df97 Address ResourceWarning warnings
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2019-11-26 18:48:01 +00:00
JoeLametta
fff3014e15 Address test failures
More details about the fix to the testDuration failure (regression):

```
FAIL: testDuration (whipper.test.test_image_toc.CapitalMergeTestCase)
testDuration
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/defer.py", line 151, in maybeDeferred
    result = f(*args, **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 221, in runWithWarningsSuppressed
    reraise(exc_info[1], exc_info[2])
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 217, in runWithWarningsSuppressed
    result = f(*a, **kw)
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py", line 271, in testDuration
    self.assertEqual(self.table.getFrameLength(), 173530)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/trial/_synctest.py", line 432, in assertEqual
    super(_Assertions, self).assertEqual(first, second, msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 829, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 822, in _baseAssertEqual
    raise self.failureException(msg)
twisted.trial.unittest.FailTest: 184930 != 173530
```

The test fails because if either nextTrack.session or thisTrack.session are None the if is false and the instructions inside it aren't executed. The check for None is needed because Python 3 doesn't allow NoneType comparisons (in Python 2 that was possible).
IIRC correctly in that test nextTrack.session has value 2 while thisTrack.session is None. That means the Python 2 version evaluates the if condition to true, while the Python 3 version in the first commit does not.
With this change both of the values of nextTrack.session and thisTrack.session are compared as int (if None, the value 1 is used for the comparison - as in disc session 1).

Regression introduced in 64dd9d843a.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2019-11-26 18:47:54 +00:00
JoeLametta
35201d5290 Address errors, improvements, formatting
- Removed unused code not portable due to buffer() use
- raw_input() does not exist in Python 3
- Fixed octal constant syntax for Python 3
- Fixed TypeError
- Replace if not exists: makedirs(path) with single call: using makedirs(path, exist_ok=True)
- Class inherits from object, can be safely removed from bases in python3: pylint's useless-object-inheritance (W0235) check

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2019-11-26 18:46:12 +00:00
JoeLametta
b2d06645af Address Freso's comment
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2019-11-26 18:46:12 +00:00
Drew DeVault
64dd9d843a Initial pass on python 3 port
Given the imminent end-of-life for Python 2, I didn't bother making the
codebase compatible with both.

Signed-off-by: Drew DeVault <sir@cmpwn.com>
2019-11-26 18:46:10 +00:00
JoeLametta
f740a0ef0d Add missing backslash to RegEX
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
2019-11-01 09:26:42 +00:00
JoeLametta
8e60fb9a18 Merge pull request #415 from itismadness/ruamel_yaml
Use ruamel.yaml for formatting and outputting rip .log file
2019-10-27 09:11:49 +01:00
itismadness
3cd2da79c3 write unit test case for WhipperLogger
Signed-off-by: itismadness <itismadness@users.noreply.github.com>
2019-10-27 08:00:00 +00:00
itismadness
862afffee9 Use ruamel.yaml for formatting rip .log file
- insert newlines after blocks of content
- strip quotes around creation time and offset
- Change offset value representation
- Clarify logger regular expressions
- Fix missing AccurateRip Summary field
- Read usage of time.gmtime(epoch) for generating datetime string
- fix missing dependency in travis
- install pycdio with specific version separately

Signed-off-by: itismadness <itismadness@users.noreply.github.com>
2019-10-27 08:00:00 +00:00
JoeLametta
d7f6d6888f Merge pull request #274 from mtdcr/accuraterip-module
accuraterip-checksum: convert to python C extension
2019-10-26 11:32:27 +02:00