Skip to content
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

[v2.2.0] Cover extraction creates invalid files #3279

Closed
DigiBC opened this issue Jul 29, 2023 · 3 comments · Fixed by #3302
Closed

[v2.2.0] Cover extraction creates invalid files #3279

DigiBC opened this issue Jul 29, 2023 · 3 comments · Fixed by #3302
Assignees

Comments

@DigiBC
Copy link

DigiBC commented Jul 29, 2023

Describe the bug

With a previous Liquidsoap development version from mid-March it was easily possible to extract cover art from metadata using the metadata.cover function. Besides MP3 and OGG metadata, FLAC metadata also worked (see #2952) and the extracted PNG and JPEG files were accurate.

In release version 2.2.0, the metadata.cover function only creates obviously invalid PNG and JPEG files that can't be displayed. Graphics software claim incorrect file headers.
Interestingly, the created files are somewhat larger than the original embedded PNG and JPEG cover art graphics.

To Reproduce

Unfortunately, I don't have a Liquidsoap developer version from that time anymore, but the following code example created accurate PNG files:

def extract_cover_art(m)
  file.write(append=false, data="#{metadata.cover(m)}", "/path/to/file.png")
end
source.on_metadata(extract_cover_art)

Expected behavior

Valid PNG and JPEG files that match the cover art graphics embedded as metadata.

Version details

  • OS: Linux Mint 21.2 (Ubuntu 22.04 'Jammy') amd64
  • Version: Liquidsoap 2.2.0

Install method

Install of official package 'liquidsoap_2.2.0-ubuntu-jammy-1_amd64.deb'
(also latest development packages)

@smimram smimram self-assigned this Jul 30, 2023
@DigiBC
Copy link
Author

DigiBC commented Aug 6, 2023

In the meantime, I seem to have found the cause of the issue:
Shortly before the release of version 2.2.0, string recoding improvements were implemented in commit aa84675, referencing issues #3231 / #3238. As a result of this change, metadata is recoded to UTF-8 by default.
This makes a lot of sense, but unfortunately the current implementation also converts binary data such as cover art images embedded in the metadata.
This leads to invalid image files when extracting cover art from metadata.

That behavior doesn't occur in earlier Liquidsoap development versions. (The last artifact tested without issue was commit 31cac52.)

As an example, here's the header of an embedded JPEG file, in the original...

Original_JPEG_Header

... and after the undesired UTF-8 recoding:

Converted_JPEG_Header

According to the help file strings_encoding.md there's a setting that could be used as a workaround for now:
settings.metadata.recode: set to false to prevent metadata from being converted to UTF-8.
However, this setting returns an error, probably because it hasn't been implemented yet. (It can't be found in the rest of the documentation either.)

@DigiBC
Copy link
Author

DigiBC commented Aug 6, 2023

After a few more tries, I finally seem to have found a workaround! 😄

Fortunately, the previously mentioned setting is available after all, but under a different name. So, the UTF-8 recoding of the metadata can be disabled as follows:
settings.request.metadata_decoders.recode := false

With this setting, cover art images will no longer be manipulated either, so the extracted PNG and JPEG files will be accurate and usable.

This workaround helps me for the time being, but that doesn't fix the issue that reasonable UTF-8 recoding shouldn't change binary image metadata.

toots added a commit that referenced this issue Aug 6, 2023
toots added a commit that referenced this issue Aug 7, 2023
github-merge-queue bot pushed a commit that referenced this issue Aug 7, 2023
toots added a commit that referenced this issue Aug 7, 2023
@DigiBC
Copy link
Author

DigiBC commented Aug 7, 2023

I can confirm that the issue has been fixed! 👍
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants