Skip to content

Sync changes from beets.mediafile - #6

Merged
arcresu merged 43 commits into
beetbox:masterfrom
arcresu:sync-beets
Apr 27, 2019
Merged

Sync changes from beets.mediafile#6
arcresu merged 43 commits into
beetbox:masterfrom
arcresu:sync-beets

Conversation

@arcresu

@arcresu arcresu commented Apr 23, 2019

Copy link
Copy Markdown
Member

My process here was to create two clones of the beets repo, and with git filter-branch reduce them to only the changes to mediafile.py and its tests respectively. I then used git format-patch to bring the changes over since the date of the last sync, resolving some small conflicts manually.

There were quite a lot of commits and I don't claim to have inspected everything carefully. The tests do still pass locally however.

ibmibmibm and others added 30 commits April 23, 2019 15:35
MusicBrainz provids composer, lyricist and arranger infomations related
to individual recordings. This commit adds query parameters to fetch them, and
write down to media files.

Tagging mapping is implemented according MusicBrainz Picard's data:
https://picard.musicbrainz.org/docs/mappings/

Signed-off-by: Shen-Ta Hsieh <ibmibmibm.tw@gmail.com>
The exception now says something about what caused it, instead of just
listing the path that led to the error.
I'm not sure how this got messed up, but this was:
- Trying to store album gain in the track peak part of the SoundCheck tag!
- Not writing the album gain to the non-SC free-form RG tag!

Together, this led to serious weirdness when writing these fields on AAC
files.

To be clear, we currently *only* support track-level data for SoundCheck. We
also record album-level RG information in non-iTunes tags, but that's
separate. A little googling suggests that SoundCheck now has album-level data,
but supporting that is a separate issue.
...when Mutagen doesn't recognize the type at all.
The regex «[\+-]?[0-9]*» possibly matches a single minus/plus, which would then
be passed on to int(), raising a ValueError from within _safe_cast. The test
suite covered this for float, but not for int.

We now make sure we actually have a number after the sign by using a Kleene
plus.
- Fixed unparseable.dsf
- Added DSF feature detection to test_mediafile.py
There isn't currently an `image.dsf`, so those tests fail.
@arcresu

arcresu commented Apr 23, 2019

Copy link
Copy Markdown
Member Author

The diffs from beets.mediafile look encouraging after this process:

diff --git a/../beets/beets/mediafile.py b/mediafile.py
index 32a32fe..99f9e01 100644
--- a/../beets/beets/mediafile.py
+++ b/mediafile.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# This file is part of beets.
+# This file is part of MediaFile.
 # Copyright 2016, Adrian Sampson.
 #
 # Permission is hereby granted, free of charge, to any person obtaining
diff --git a/../beets/test/test_mediafile.py b/test/test_mediafile.py
index 36a2c53..1a1192d 100644
--- a/../beets/test/test_mediafile.py
+++ b/test/test_mediafile.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# This file is part of beets.
+# This file is part of MediaFile.
 # Copyright 2016, Adrian Sampson.
 #
 # Permission is hereby granted, free of charge, to any person obtaining
@@ -26,7 +26,7 @@ import unittest
 from six import assertCountEqual
 
 from test import _common
-from beets.mediafile import MediaFile, Image, \
+from mediafile import MediaFile, Image, \
     ImageType, CoverArtField, UnreadableFileError
 
 
@@ -968,5 +968,6 @@ class MediaFieldTest(unittest.TestCase):
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 
+
 if __name__ == '__main__':
     unittest.main(defaultTest='suite')
diff --git a/../beets/test/test_mediafile_edge.py b/test/test_mediafile_edge.py
index 8bf9e19..8d31de0 100644
--- a/../beets/test/test_mediafile_edge.py
+++ b/test/test_mediafile_edge.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# This file is part of beets.
+# This file is part of MediaFile.
 # Copyright 2016, Adrian Sampson.
 #
 # Permission is hereby granted, free of charge, to any person obtaining
@@ -24,7 +24,7 @@ import mutagen.id3
 
 from test import _common
 
-from beets import mediafile
+import mediafile
 import six

@arcresu

arcresu commented Apr 23, 2019

Copy link
Copy Markdown
Member Author

Looks like some sort of problems with the CI config/infrastructure. Maybe this is something that was seen previously on beets itself?

@sampsyo

sampsyo commented Apr 23, 2019

Copy link
Copy Markdown
Member

Wow; awesome work!

I'm a little stumped by those CI failures. The job seems to fail right after trying to install Tox, but there's no obvious error in the log. It could be that stealing the Travis config from beets itself might work.

@arcresu

arcresu commented Apr 24, 2019

Copy link
Copy Markdown
Member Author

After some trial and error porting CI config from beets, this is now working! I removed the mp3gain dependency since it doesn't exist in Xenial (or in Debian where it was removed) and it didn't appear to be necessary in the first place.

@arcresu

arcresu commented Apr 26, 2019

Copy link
Copy Markdown
Member Author

Once this is merged it might be nice to push out a new release of MediaFile on PyPi so that we can start making beets depend on that. Would you like help with making/publishing that release when it comes time?

@sampsyo

sampsyo commented Apr 26, 2019

Copy link
Copy Markdown
Member

Yes; that sounds awesome! I'll need to add you (or others) as package administrators on PyPI. Do you have a PyPI username?

@arcresu

arcresu commented Apr 27, 2019

Copy link
Copy Markdown
Member Author

Great! My username on PyPi is the same as on GitHub. My plan would be to make the mediafile release shortly after the next beets release in order to introduce the dependency early in the next beets release cycle.

I'm aware of at least one active PR in beets that affects mediafile. My plan would be to start by checking open PRs there that affect mediafile and try to help move them to completion if possible and assist with translating the changes to this repository.

@arcresu
arcresu merged commit ea01b1f into beetbox:master Apr 27, 2019
@arcresu
arcresu deleted the sync-beets branch April 27, 2019 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.