WavPack DSD tags don't work when embedded artwork is used

[986ster]

If a WavPack DSD file (.wv) is tagged with APEv2 tags then the scanner reads them properly and everything behaves as it should, unless an embedded image is used (which APEv2 supports), in which case the scanner fails to read all the tags.

Presently WavPack DSD files have to use an external image file alongside the .wv files in order to get tags and album art properly working in LMS.

[986ster]

More details in forum post: https://forums.slimdevices.com/forum/user-forums/logitech-media-server/1650309-wavpack-dsd-tags-not-recognized-with-embedded-artwork-files-sent-to-players-as-pcm

[michaelherger]

Would you have a small example file to test this with? Is this limited to DSD files inside WavPack, or any WavPack? In the latter case it might be simpler to get a test file.

[986ster]

I've posted two WavPack DSD files at the following location (one with an embedded image, one without): https://drive.google.com/drive/folders/1ede86Wp7ZlEMzCi35LJbmWAGderTZaNE?usp=drive_link Could you let me know when you have them, so I can remove them.

I don't know if this is impacting only WavPack DSD, or all WavPack files, as WavPack PCM is not a format I've had any interest in using before. I just wanted to see if it the WavPack DSD container made sense now that I'm getting more DSD files in order to have a compressed format to keep them in, much like I use FLAC for PCM.

[986ster]

If you need smaller files than these, I could convert a really short PCM file to DSD and then package it up in WavPack.

[michaelherger]

Thanks for those files. What I'm seeing is this:

Warning: [23:06:15.1824] APE: [Tag larger than maximum possible size]

That seems to come from the APE scanner, which defines a maximum size of 100kB per tag: https://github.com/andygrundman/Audio-Scan/blob/fc617aa26899cc6591f8fe3d7b448956974d14d5/include/ape.h#L40C42-L40C42

Your image seems to be larger than that.

Maybe it would be good enough to increase that limitation and re-compile. @ralph-irving might know better.

[986ster]

That's interesting. I didn't see any particular restriction mentioned on APEv2 image tags when converting/tagging the files, and I have two applications (mp3tag and EZ CD Audio Converter) that both read and write these tags without issue. I wonder if this either wasn't a supported use case or if there was a size limitation in the specification that's subsequently been removed back when the scanner code was developed.

[986ster]

From what I can see the specification for the tag size is a 32-bit value, so it can support up to 4GB per tag, despite the author recommending an 8KB limit and stating that he hates the idea of images being stored in tags. If you want a tag limit of 8KB and hate images then why would you allow the size to be stored as a 32-bit value and also allow binary data to be stored?!

[michaelherger]

Would you be able to build the binary with the help of buildme.sh in https://github.com/Logitech/slimserver-vendor/tree/public/8.4/CPAN to give this a try?

[986ster]

I actually need to build another linux box over the coming days for something else, so I'll be able to install LMS, the compiler, and other prerequisites on that box to be able to test this first, which will allow me to keep the build totally clean on my main box. I didn't see any new files in the CPAN path, so do you need me to manually update Andy's original header file to allow a larger tag size?

[michaelherger]

There are more up-to-date copies of Audio::Scan in that repository, provided by @ralph-irving. extract the latest (I think it's 1.08), modify the header file, then use the script to build only that module.

[michaelherger]

FWIW: I gave this a quick try on my Mac, increasing the max size to 1MB instead of 100kB, and it worked as expected:

The image in there seems to be about 169kB.

[986ster]

That's great news. Is there a maximum image size defined across LMS, because it would appear sensible to set the max size in the APE scanner to this value rather than the permissible 4GB or just some arbitrary value, if we run the risk of causing problems downstream otherwise? I guess the most ideal solution would have the scanner ignore any one tag that exceeded the max size rather than ignore all tags, but I'm not sure it's worth the development at this stage if this format is so niche and the other people that also noticed the issue just worked around it by using external image files.

[michaelherger]

There's no LMS wide limit.

While I do agree that the scanner shouldn't fail 100% because of one oversized tag. But fixing that is way beyond my C.

[terual]

The check of the size limits happens in the validation step, I don't think we should tinker too much there. If validation fails the whole scanning should fail I think. We should not want to process malformed tags.

BTW, interesting is the following statement from the APEv2 tag specification: https://github.com/taglib/taglib/blob/master/taglib/ape/ape-tag-format.txt#L29C1-L35C1. But let's see how the 2MB limit works out.

It is strongly recommended that the data size be stored in the tags.  The size
should normally be in the roughly one kilobyte, never more than 8 kilobytes.

Larger data should be stored externally using link entries.  Linked data is much
easier to process by normal programs, so for instance JPEG data should not be
included inside the audio file.

[michaelherger]

Yeah... see the comment in https://github.com/LMS-Community/Audio-Scan/blob/master/include/ape.h#L40

So... strictly speaking: @986ster, don't store artwork in the files. It's wrong according to spec 😉.

[986ster]

Yeah... see the comment in https://github.com/LMS-Community/Audio-Scan/blob/master/include/ape.h#L40

So... strictly speaking: @986ster, don't store artwork in the files. It's wrong according to spec 😉.

It's true that the developer wrote those comments, but they were back in 2004. and the broader context was he expected the tags to be repeated at intervals through the files to allow for streaming, so that's why he was obsessed with them being as small as possible in order to not interrupt streaming at 2004 data rates. He also thought it was a terrible idea to store the album cover for each track, because it would waste space across an entire album, which is a non-issue in 2023 when the size of a JPEG is compared to even the most basic solid-state storage available now.

If he wasn't planning for the tags to be extensible then he shouldn't have allocated a 32-bit value for the tag size, allowing 4GB, and also allowing binary data types. Every tagging program that supports APEv2 tags allows reasonable size images in the files, so I really don't think we're going against the grain with this enhancement.

I haven't had an opportunity to compile the scanner myself yet, because I've needed my LMS environment up and running to test something else, but I'm good with the 2MB limit. I checked the largest images I have across my entire collection and they max out at under 800K for the most complex images, so there's no way the additional textual tags would take this anywhere near 2MB.

[986ster]

@michaelherger I finally got this to work. I ended up building 1.06 with the APE header value changed, so I could just swap out the shared library, as the later versions wouldn't play ball this way.

With that change the tags worked properly for WavPack PCM and DSD files, including the artwork, so I think we're good to go with this.

[986ster]

Since there are a number of different versions of Audio:Scan out in the wild, and different distros seem to get different versions from what I can see in philippe44's forum post about the same (why?), any thoughts on how we can get this packaged into a deployable version? I compiled an aarch64 version to prove the concept, as I have a test LMS server available on a Raspberry Pi where I can do builds, but I don't have the compiler installed on my x64 production machine.

[michaelherger]

What do you mean by "packaged into a deployable version"? Build for LMS, or build for Linux distributions?

I have a Github Workflow to build for a few more recent Linux platforms. But to be honest: I haven't seen a need to distribute the latest changes widely, as they're really covering rare corner cases. Yes, it would be great to be on the same level on all platforms. But it's a lot of work...

[986ster]

I meant the LMS builds that include this executable, so this would be Linux and other platforms. I would have thought that when you downloaded the latest nightly LMS build you'd also get the latest version of the Audio:Scan executable, but it appears that either different platforms and/or different distributions are getting or are choosing to use different versions for whatever reason. I take it then that Audio:Scan doesn't get rebuilt as part of the process that deploys the nighly versions, and this is something you have to manage outside that process when there are changes significant enough to warrant it? If that's the case then I can park this until such time as we're in a position for a big update to it that'll warrant a recompile for the different platform types.

[michaelherger]

Yes, exactly: those dependencies aren't built automatically. But we've covered "your" package, haven't we?

[986ster]

Yes, exactly: those dependencies aren't built automatically. But we've covered "your" package, haven't we?

Yes. I'll need to compile it for my x86-64 platform, but I can do that now that I know not to expect it just to appear in one of the nightly builds.

[michaelherger]

I'm working on improving the build pipeline, which should simplify addition of those binaries. But I can't guarantee a delivery time...

[michaelherger]

Yes, exactly: those dependencies aren't built automatically. But we've covered "your" package, haven't we?

Yes. I'll need to compile it for my x86-64 platform, but I can do that now that I know not to expect it just to appear in one of the nightly builds.

What Perl version would you need?

[986ster]

What Perl version would you need?

This is what I'm running on my production instance: Perl Version: 5.36.0 - x86_64-linux-gnu-thread-multi Audio::Scan: 1.06

[michaelherger]

Could you please give this a try?

CPAN-debian-bookworm.zip

Took about three clicks and a minute to build using my Github workflow 😉.

[986ster]

Thanks. I'm away at the moment, but I'll try this once I'm back home and have my main system available.

[986ster]

@michaelherger I got to try this today. It doesn't launch at all. You click Rescan, and then nothing happens, and there's nothing written to the scanner log.

This is the same sort of behaviour I saw when I was trying to compile 1.09 on the Raspberry Pi version I have for testing, and ended up having to compile 1.06 before it would work. Your version is 1.06, so I have no idea why it's not working, as it's the correct Perl version and architecture.

[michaelherger]

You might have to replace or edit Scan.pm: there's a version string in there. This must be 1.09 rather than 1.06

[986ster]

Was the version you compiled 1.09? For some reason I thought the zip file said it was version 1.06, but I can't find any reference to that any longer, so I may have totally made that up.

[michaelherger]

I think it was 1.09, yes. Would you have a Scan.pm in that archive?...

[986ster]

I have the 1.09 Scan.pm downloaded for my RPi test environment, that I got to work with the compiled 1.09 Scan.so now that I know there's a dependency between them. I'll try installing that on my main Intel environment later alongside the Scan.so I got from you, because that's likely the problem then.

[986ster]

The 1.09 Scan.pm solved the problem. This is great, because it also includes the other scanning improvements since 1.06, particularly the seeking in Opus files that works so much better now. Not a filetype I'm particularly fond of, but it's a demonstrable improvement.

[986ster]

I'm going to add that I've confirmed that this also fixes the same tagging problem on Monkey's Audio tracks (which also use APE tags) and the PCM version of WavPack too, so this is a pretty solid benefit to a number of file formats that LMS supports.

[philippe44]

BTW, I think that seeking is still broken there because the current version, even with these improvements did not handle the fact that when a page has no packet terminating into it, its granule is -1. It was sending the seek totally off. I think I've fixed that in 1.10

[986ster]

I did see that you had a PR out there for 1.10.

I'm not at all invested in Opus as a local storage format, so it's not a big deal for me, but it's nice to see that there's progress being made on closing out some of these niggling issues that remain out there. I just happen to keep a test suite of different formats so I can validate that everything is working as expected when making changes, and I did happen to notice that the Opus scanning had definitely been improved over 1.06. Sounds like it's going to get better yet with 1.10.

[philippe44]

If by chance you have tests for OggFlac, I could use some help

[986ster]

I tried creating an OggFlac file earlier, because I saw you were working on adding this format to LMS. The trouble is neither my ripping/conversion software, nor my preferred tagging software support this format. I was able to create what I think is an OggFlac file using the flac executable with the --ogg parameter, but I have no means of validating it or adding tags to get an idea of whether it's even a legitimately playable file.

[philippe44]

yes, I know that for example Mp3Tag does not support it. Candidly, the most important use was some OggFlac radio, and even that is a corner case (talking about corner cases and the ones one likes or not... 😄).

If you want to try OggFlac files in LMS, you need to add it to types.conf and then use the updated Audio::Scan. I can provides it for your platform, but I can't say with a straight face this is a very important topic.

OggFlac as a streaming protocol is supported, unless the webradio updates metadata during the stream (it's called Ogg "chaining") and that's what started all these changes I've done even to flac itself.

[986ster]

Actually, I just discovered that if I give it a .ogg extension I can read/edit the tags in EZ CD Audio Converter, so I may be able to help with this. Let's take this particular conversation over to the 1.10 PR so that we can keep this one focused on the WavPack functionality.

[philippe44]

Indeed