beetbox / pyacoustid

Python bindings for Chromaprint acoustic fingerprinting and the Acoustid Web service
MIT License
332 stars 66 forks source link

Failing to encode on Windows #24

Closed philipbjorge closed 2 years ago

philipbjorge commented 10 years ago

Is there a reason we're not retrying with a system encoded version of the path when we hit the subprocess bug (like in the proposed patch for the python bug)? This fixes fingerprinting of unicode paths on my Windows box.

https://github.com/sampsyo/pyacoustid/blob/master/acoustid.py#L285 http://bugs.python.org/file11674/Python-2.5.2-subprocess.patch

Thoughts on the following fix?

  1. Retry the popen in the UnicodeEncodeError exception block with encoded paths
  2. Reraise the "argument encoding failed" exception for any failures during the retry

If you like it I'll send a pull request your way.

sampsyo commented 10 years ago

Really? This is as simple as using a bytestring path? I didn't think that was possible on Windows! Awesome; let's definitely do it if it works.

Two questions:

philipbjorge commented 10 years ago

So it's definitely working locally on my machine (where it was failing before).

  1. Yes, I don't see why this shouldn't be possible.
  2. Unsure -- I can try and figure out. Would this just entail testing with a path > MAX_PATH?

It'll be a couple days until I get to this probably.

sampsyo commented 10 years ago

Yes, just testing with a long (unicode) path should do awesomely.