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

CUERipper: Detect too large album art earlier #311

Conversation

c72578
Copy link
Collaborator

@c72578 c72578 commented Jan 20, 2024

The maximum size of embedded album art is 16 MB each.
So far, in case of too large album art, an exception was shown after
ripping the CD, when tagging the files: "Block size too large."
When using libFLAC, an exception appeared at the beginning of ripping
"Exception: unable to initialize the encoder:"
FLAC__STREAM_ENCODER_INIT_STATUS_ENCODER_ERROR

  • Show an earlier warning, after downloading too large album art
    and before starting to rip the CD.
  • Switch to Album art size Small, if the album art is larger than
    16 MB and Embed album art is set.
  • The following warning is shown:
    "Selected album art has a size of x bytes, which is too large for
    embedding (max 16 MB). Small album art will be used instead."
  • This is a follow-up to commit gchudov/taglib-sharp@8590ad0
    Prevent taglib from corrupting flac files when embedded album art
    exceeds 16Mb
  • Use Properties.Resources for the title and message in the new
    MessageBox "Album art too large" to allow translation.
  • Show "Downloading album art..." in the status of CUERipper to inform
    the user, what is going on, because downloading of large album art
    may take some time.
  • Add German translations.

The maximum size of embedded album art is 16 MB each.
So far, an exception was shown after ripping the CD, when tagging
the files: "Block size too large."

- Show an earlier warning, after downloading too large album art
  and before starting to rip the CD.
- Switch to `Album art size` `Small`, if the album art is larger than
  16 MB and `Embed album art` is set.
- The following warning is shown:
  "Selected album art has a size of x bytes, which is too large for
  embedding (max 16 MB). Small album art will be used instead."
- This is a follow-up to commit gchudov/taglib-sharp@8590ad0
  Prevent taglib from corrupting flac files when embedded album art
  exceeds 16Mb
@c72578
Copy link
Collaborator Author

c72578 commented Jan 20, 2024

Here is a build for testing:
CUETools_2.2.5_2024-01-20_git_e71bbfd_CUERipper_Detect_too_large_album_art_earlier.zip
SHA256: 8380b15c33440cd87f8a2272d99cadc60c23ebc33a0377d7d5ff72308fdc1f85

If the selected album art is larger than 16 MB and the option "Embed album art" is set, then the following warning is shown:

Album_art_too_large

@ha-korth
Copy link

Art size dropped from 39.9 MB to 19.6 KB so works.
I'd still like to see MaxAlbumArtSize= active (maybe ignored if blank or 0) but that might just be me.

- Use Properties.Resources for the title and message in the new
  MessageBox "Album art too large".
- Show "Downloading album art..." in the status of CUERipper to inform
  the user, what is going on, because downloading of large album art
  may take some time.
- Add German translations.
@c72578
Copy link
Collaborator Author

c72578 commented Jan 21, 2024

Art size dropped from 39.9 MB to 19.6 KB so works. I'd still like to see MaxAlbumArtSize= active (maybe ignored if blank or 0) but that might just be me.

@ha-korth Thanks for testing.
The goal of this PR is to detect too large album art earlier and inform the user.
Considering MaxAlbumArtSizealso in CUERipper would be a separate modification.

@c72578
Copy link
Collaborator Author

c72578 commented Jan 21, 2024

A new test build has been prepared:
CUETools_2.2.5_2024-01-21_git_8df5575_CUERipper_Detect_too_large_album_art_earlier.zip
SHA256: e562b589ad36a07255fd04bc874f577c1a23daaf7aaf8e2c476c3e4563d53cb6

In addition to showing a message in case of too large album art, "Downloading album art..." is shown in the status of CUERipper while downloading, to show what is going on, as downloading of large album art may take some time. See green arrow:

CUERipper_Status_Downloading_album_art_(green_arrow)

@ha-korth
Copy link

Mine shows as CUERipper 2.2.5 (Not Responding) while art is downloaded but at least the Downloading album art... explains why the program is waiting.

@c72578
Copy link
Collaborator Author

c72578 commented Jan 22, 2024

Mine shows as CUERipper 2.2.5 (Not Responding) while art is downloaded but at least the Downloading album art... explains why the program is waiting.

Yes, this is the reason, why the status message "Downloading album art..." has been added now.
In the past the behavior while the large album art is fetched was described e.g. as:

hangs for about 5minutes, then starts ripping the disc

https://hydrogenaud.io/index.php/topic,121449.0.html

@c72578 c72578 merged commit 5004df7 into gchudov:master Feb 3, 2024
1 check passed
@ha-korth
Copy link

ha-korth commented Feb 16, 2024

Found another issue with larger album art in CUERipper. Just wanted to make a note of it somewhere.
https://ia800504.us.archive.org/32/items/mbid-d24b2001-ce3c-4fec-b308-9d7d113b0f2d/mbid-d24b2001-ce3c-4fec-b308-9d7d113b0f2d-36648174305.jpg
6.52 MB
Tested both cuetools flac encoder and libFLAC encoder.
No flac tag information shown in Windows 11 file explorer.
Media Player in Windows 11 throws error.
cover
flac -t shows files ok
files play in other players e.g. foobar2000 and tags/cover are ok
cover2

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.

2 participants