-
Notifications
You must be signed in to change notification settings - Fork 374
Set file timestamp to pubdate and save more id3 information #213
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
Conversation
Fix issue scdl-org#166 Inspired from youtube-dl Not tested on Windows but it work on OSX and probably on Unix based system.
|
Hi ! thanks for your PR ! Unfortunately your patch is crashing on my laptop. |
scdl/scdl.py
Outdated
| logger.debug(e) | ||
| else: | ||
| logger.error("This type of audio doesn't support tagging...") | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have trailing whitespace here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! My bad. It's totally unprofessional.
My first and last time I submit patch directly with the github editor code.
Specially when it's late in the night >_<
| logger.info('\nTrack n°{0}'.format(counter)) | ||
| download_track(track) | ||
|
|
||
| def try_utime(path, filetime): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you wrapping this in a try catch ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was inspired from youtube-dl.
But in general using a try catch will prevent breaking change of your code. And like I said I did not test it on Windows.
scdl/scdl.py
Outdated
| audio['COMM'] = mutagen.id3.COMM(encoding=3, text=track['description']) | ||
| audio['TYER'] = mutagen.id3.TYER(encoding=3, text=track['created_at'][:4]) | ||
| audio['WOAS'] = mutagen.id3.WOAS(url=track['permalink_url']) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace.
|
|
||
| #Try to change the real creation date | ||
| created_at = track['created_at'] | ||
| filetime = int(time.mktime(datetime.strptime(created_at, '%Y/%m/%d %H:%M:%S %z').timetuple())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
datetime doesn't seem initialized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have a look at 0736cb0#diff-834ccb1f68cfeac6d63b0b550a320ba0R467 for inspiration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing import.
from datetime import datetime
scdl/scdl.py
Outdated
| #Try to change the real creation date | ||
| created_at = track['created_at'] | ||
| filetime = int(time.mktime(datetime.strptime(created_at, '%Y/%m/%d %H:%M:%S %z').timetuple())) | ||
| try_utime(os.path.join(os.getcwd(), filename),filetime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use relative path here :)
try_utime(filename, filetime)
|
There is still trailing whitespace. Also the id3 tag editing doesn't seems to work. You can have a look at #147. |
devalnor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and working well on OSX.
You can check ID3 tags results with eyed3 (nice tool btw)
|
|
||
| #Try to change the real creation date | ||
| created_at = track['created_at'] | ||
| filetime = int(time.mktime(datetime.strptime(created_at, '%Y/%m/%d %H:%M:%S %z').timetuple())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing import.
from datetime import datetime
| else: | ||
| logger.error('Artwork can not be set.') | ||
| audio.save() | ||
| audio.save(v2_version=3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed TagID to 2.3 instead of 2.4.
More stable and fix the date issue:
https://stackoverflow.com/questions/18026516/some-mutagen-tags-dont-work
| shutil.copyfileobj(response.raw, out_file) | ||
| out_file.seek(0) | ||
|
|
||
| track_date = datetime.strptime(track['created_at'], "%Y/%m/%d %H:%M:%S %z") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inspired from @sublimo's fork.
|
looks good :) |
You need to add |
|
Audio source URL are not encode in utf-8 why do you want to do that? |
|
The string is a python bytecode object. That's why there is a |
|
@flyingrub after testing, I can't reproduce your issue. Is there something that I missing or can you explain me more the issue? |
|
OK. So I was using eyeD3 with python3 and it seems that it doesn't support well the encoding when it run on python3. After using it on python2 it works well. |
Fix issue #166
Inspired from youtube-dl
Not tested on Windows but it work on OSX and probably on Unix based system.