Support for DSF files

[docbobo]

Adds support for DSF Files to beets, based on the work in quodlibet/mutagen#283. This should resolve #459.

Although this works locally, I am not sure if this is all that needs to be done. So please review and let me know if there will be additional changes required.

Please do not merge before quodlibet/mutagen#283 has been accepted.

[sampsyo]

Awesome! This looks good so far.

Would you mind also adding a test file to test/rsrc and associated boilerplate to test/test_mediafile.py?

Also, one thing we've done in the past for Mutagen dependencies is to use feature detection. That way we can release a beets version with the new support before the code is even merged into Mutagen. That's not necessary, of course—if the Mutagen side is ready soon, we can just bump our version number requirement.

[docbobo]

I did add the requested tests to test/test_mediafile.py, though I am not completely sure that I created the correct test/rsrc/unparseable.dsf - perhaps you have some background on that?

Regarding the feature detection: where would I have to look for more details on how that works?

[sampsyo]

Things look great so far! I'll check out the test now.

For this particular extension, I actually don't know if any code is necessary at all to maintain compatibility. I don't think the import mutagen.dsf line is necessary; Mutagen kindly does the file type detection for us. See b9500c3, where I've removed all the unnecessary Mutagen imports. And without that, I think things should "just work" before and after the support is added to the library.

For the tests, we might do something like:

try:
    import mutagen.dsf
except:
    HAVE_DSF = True
else:
    HAVE_DSF = False

followed by @unittest.skipIf(not HAVE_DSF) on the tests in question.

[sampsyo]

I know it's not quite the expansion of the acronym, but maybe "DSD Stream" or even just "DSD" would better match the other short names we have here, none of which have a "file" or "audio" suffix?

[docbobo]

Well, there is also DSDIFF. Although it doesn't support tags, it might be valuable to be as precise as possible here. But if you want me to change that, I'll happily do that.

[sampsyo]

Arg, you're right—it's weird that there are competing standards here. But I agree in that case; let's keep the full name.

[sampsyo]

For the unparseable test, it looks like the intent is to include a date field that's empty (instead of committing the date file altogether). Here, for example, are the contents for unparseable.mp3:

>>> mutagen.File('test/rsrc/unparseable.mp3')
{'TDRC': TDRC(encoding=<Encoding.UTF8: 3>, text=[u''])}

It looks like the current test file has no tags at all:

>>> mutagen.File('test/rsrc/unparseable.dsf')
{}

Maybe the test file should just use the same empty-string frame as the MP3?

I realize this is confusing and poorly documented—I'm going to add some comments to the source now to make this more intelligible.

[docbobo]

Ok, I've "fixed" the unparseabl.dsf and also added the DSF feature toggle to the test case. Let me know if there's anything else.

[docbobo]

And let me know if you want me to rebase everything into one commit before merging the PR...

[sampsyo]

Looks great; thanks! I merged this and did some tests locally, so we should be good to go.

I had to turn off the image tests because there isn't an image.dsf test fixture file yet. We can re-enable those, of course, if you think it would be helpful to add that fixture too.

[docbobo]

I do have the image.dsf here. Do you believe there is value in running those tests? It's pretty much basic ID3.

[sampsyo]

This sounds a bit ambivalent, but I'll leave that up to you—if you think there's anything that might be revealed about the DSF format with images (e.g., when tags get much larger than in the plain-text case), then let's do it. But if you think it's totally redundant with the MP3 tests, there's no need.

[docbobo]

Ok. Let's keep it the way it is then.

[sampsyo]

Sounds good! Thank you again for all your work on this. ✨