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

Unicode issues #215

Closed
dnalor opened this issue Jan 26, 2018 · 6 comments
Closed

Unicode issues #215

dnalor opened this issue Jan 26, 2018 · 6 comments
Assignees
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Needed: tests Tests are required Priority: low Low priority Sprintable Small enough to sprint on Stretch Optional goal for the current sprint (may not be delivered)
Milestone

Comments

@dnalor
Copy link

dnalor commented Jan 26, 2018

  • It seems the catalog number can be non-ascii, so there was a encoding error in program.py.
    Fix: common/program.py line 370:
                if metadata.catalogNumber:
                    self._stdout.write("Cat no  : %s\n" %
                                       metadata.catalogNumber.encode('utf-8'))
Ripping track 5 of 10: 05 - Gripped by Fear.flac
CRCs match for track 5                    
Peak level: 99.52% 
Rip quality: 100.00%
Traceback (most recent call last):
  File "/home/user/appdata/venv-whipper/bin/whipper", line 11, in <module>
    load_entry_point('whipper==0.5.1', 'console_scripts', 'whipper')()
  File "/home/user/appdata/venv-whipper/lib/python2.7/site-packages/whipper-0.5.1-py2.7.egg/whipper/command/main.py", line 40, in main
    ret = cmd.do()
  File "/home/user/appdata/venv-whipper/lib/python2.7/site-packages/whipper-0.5.1-py2.7.egg/whipper/command/basecommand.py", line 124, in do
    return self.cmd.do()
  File "/home/user/appdata/venv-whipper/lib/python2.7/site-packages/whipper-0.5.1-py2.7.egg/whipper/command/basecommand.py", line 124, in do
    return self.cmd.do()
  File "/home/user/appdata/venv-whipper/lib/python2.7/site-packages/whipper-0.5.1-py2.7.egg/whipper/command/cd.py", line 190, in do
    self.doCommand()
  File "/home/user/appdata/venv-whipper/lib/python2.7/site-packages/whipper-0.5.1-py2.7.egg/whipper/command/cd.py", line 479, in doCommand
    _ripIfNotRipped(i + 1)
  File "/home/user/appdata/venv-whipper/lib/python2.7/site-packages/whipper-0.5.1-py2.7.egg/whipper/command/cd.py", line 375, in _ripIfNotRipped
    if os.path.exists(path):
  File "/home/user/appdata/venv-whipper/lib64/python2.7/genericpath.py", line 26, in exists
    os.stat(path)
UnicodeEncodeError: 'latin-1' codec can't encode character u'\u25b7' in position 69: ordinal not in range(256)

Other examples:
Jean-Michel Jarre - Revolutions / Track 3 / https://musicbrainz.org/release/e2d4719c-34d2-445f-a1f1-c6877b614411
R.E.M. - Green / Track 8 (Character u'\u2010')

Fix: command/cd.py line 362:

            logger.debug('ripIfNotRipped: path %r' % path)
            trackResult.number = number
# convert
            path = unicode( path.encode('latin1','ignore'), 'latin1', 'ignore')
@JoeLametta JoeLametta added the Bug Generic bug: can be used together with more specific labels label Jan 26, 2018
@JoeLametta JoeLametta added this to the 1.0 milestone Apr 6, 2018
@JoeLametta JoeLametta self-assigned this Apr 6, 2018
@JoeLametta JoeLametta added the Status: in progress Issue/pull request which is currently being worked on label Nov 2, 2018
@JoeLametta
Copy link
Collaborator

JoeLametta commented Nov 2, 2018

@Freso You're the MusicBrainz expert here: could you please comment about the non ASCII catalog number issue?

  • Fix catalog number issue
  • Check if the track names issue still exists in the latest whipper version

@Freso
Copy link
Member

Freso commented Nov 2, 2018

What kind of comment do you want? I don't think MusicBrainz puts arbitrary restrictions on what characters can go into a cat. no., and AFAIK all text is stored in the MB db as UTF-8 (and served as such too).

@JoeLametta
Copy link
Collaborator

JoeLametta commented Nov 2, 2018

@Freso Thanks for your confirmation: didn't know if it was "normal" to have a non ASCII catalog number.

@Freso
Copy link
Member

Freso commented Nov 2, 2018

It isn't. I haven't encountered it before. :) I'm not saying it's normal, I'm just saying it isn't "out of spec".

@JoeLametta
Copy link
Collaborator

@Freso It seems that the track names issue is no longer present in latest whipper version... (otherwise I suppose #319 would have failed before).

Fix included in: #323.

@JoeLametta JoeLametta added completed and removed Status: in progress Issue/pull request which is currently being worked on labels Nov 2, 2018
@Freso
Copy link
Member

Freso commented Nov 2, 2018

Yeah, I've ripped tons of discs with non-ASCII release names, artist names, track names, etc. They all work fine. I don't think I've ever encountered one with a non-ASCII cat. no. though. :)

@JoeLametta JoeLametta added Status: in progress Issue/pull request which is currently being worked on Accepted Accepted issue on our roadmap Priority: low Low priority Sprintable Small enough to sprint on Needed: tests Tests are required Stretch Optional goal for the current sprint (may not be delivered) and removed completed labels Nov 11, 2018
@JoeLametta JoeLametta removed the Status: in progress Issue/pull request which is currently being worked on label Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Needed: tests Tests are required Priority: low Low priority Sprintable Small enough to sprint on Stretch Optional goal for the current sprint (may not be delivered)
Projects
None yet
Development

No branches or pull requests

3 participants