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

Segmentation fault when adding album art #14

Open
subatomicglue opened this issue Jan 13, 2021 · 9 comments
Open

Segmentation fault when adding album art #14

subatomicglue opened this issue Jan 13, 2021 · 9 comments

Comments

@subatomicglue
Copy link

subatomicglue commented Jan 13, 2021

When tagging m4a files with certain album art,
I get files that look like this:
Folder-resized-4446.jpg

Resolution is 1583x1393

The art does not seem to be written to the .m4a file
AtomicParsely dies with:

$  AtomicParsley "track.m4a" -o __temp2435789234759.m4a --artwork Folder.jpg

Segmentation fault: 11

AtomicParsley version is from brew install AtomicParsley on MacOS

$ AtomicParsley -v
AtomicParsley version:   (utf8)

$ brew install AtomicParsley
Warning: atomicparsley 20200701.154658.b0d6223 is already installed and up-to-date
To reinstall 20200701.154658.b0d6223, run `brew reinstall atomicparsley`

Confirmed that the latest GitHub version produces the same error on MacOS

$ git clone https://github.com/wez/atomicparsley.git
$ mkdir build
$ cd build
$ cmake ..
$ make -j 4
$ ./AtomicParsley "track.m4a" -o __temp2435789234759.m4a --artwork Folder.jpg
Segmentation fault: 11
$ ./AtomicParsley -v
AtomicParsley version: 20201231.092811.0 cbecfb1e2238b7c4261074725d25cbb297656c20 (utf8)

Actual files discussed above (I own the (c) rights to these):
Folder-resized-4446.jpg
folder.jpg
track.m4a.zip

@github-actions
Copy link

Thanks for filing an issue! Please note that this project is only passively maintained, so your best bet for getting an issue resolved is through a pull request that is easy to verify!

@subatomicglue
Copy link
Author

subatomicglue commented Jan 14, 2021

Got this code setup in vscode for debugging... Here's the details.

Appears to be crashing in nsimage.mm, Objective C code, in this function:
bool ResizeGivenImage(const char* filePath, PicPrefs myPicPrefs, char* resized_path) {

Call stack seems to point to a strncmp at the rootcause of this.
But the code in AtomicParsley crashing is image unlockFocus with Exception has occurred. EXC_BAD_ACCESS (code=1, address=0x0):

libsystem_platform.dylib!_platform_strncmp (Unknown Source:0)
CoreFoundation!___forwarding___ (Unknown Source:0)
CoreFoundation!__forwarding_prep_0___ (Unknown Source:0)
AtomicParsley!ResizeGivenImage(char const*, PicPrefs, char*) (/Users/<username>/src/atomicparsley/src/nsimage.mm:235)
AtomicParsley!APar_MetaData_atomArtwork_Set(char const*, char*) (/Users/<username>/src/atomicparsley/src/parsley.cpp:2943)
AtomicParsley!real_main(int, char**) (/Users/<username>/src/atomicparsley/src/main.cpp:2239)
AtomicParsley!main (/Users/<username>/src/atomicparsley/src/main.cpp:4477)
libdyld.dylib!start (Unknown Source:0)

Screenshot here:
Screen Shot 2021-01-14 at 4 39 54 PM

@subatomicglue
Copy link
Author

subatomicglue commented Jan 15, 2021

I found a workaround to avoid the segfault.

When I have a jpg at 300 DPI, then AtomicParsley segfaults. If I simply change to 72, then it runs without segfault.
I am now pre-processing album-art images using ImageMagick:

convert -units PixelsPerInch Folder.jpg -density 72 NewFolder.jpg

For me, I also like to resize:

convert -units PixelsPerInch Folder.jpg -resize 500x500\!\> -density 72 NewFolder.jpg

... omit the \! to keep aspect ratio... :)

wez added a commit that referenced this issue Jan 15, 2021
- Fixes up NSImage to use a technique that doesn't depend on the
  DPI of the current screen and just uses pixels
- Fixes a crash when releasing the image; the crash was caused by
  a `memset` call getting a different size for the filename buffer.
  I solved this by passing the size down, which is safer than just
  assuming anyway.  I added an ASAN cmake option to make it easier
  to turn up the debugging and understand where the memory corruption
  had come from.

refs: #14
@wez
Copy link
Owner

wez commented Jan 15, 2021

Thanks for getting a stack trace; I've pushed a commit that seems to make things better!

PS: I enjoyed listening to your track!

@subatomicglue
Copy link
Author

subatomicglue commented Jan 15, 2021

Oh hey, that's great! Thank you so much for that! I'm automating generation of my catalog from source .wavs, this tool is great. Really appreciate this!

(I’ll try out the patch tomorrow, and report back)

@subatomicglue
Copy link
Author

I tested and can confirm that this works, thanks!

Will this go into the homebrew/brew repository?

@wez
Copy link
Owner

wez commented Jan 15, 2021

I've tagged a new release (20210114.184825.1dbe1be) that has this change.

I don't directly maintain the brew recipe; if you feel like it, you could submit a PR to brew to have them pick up the new version (it would look something like Homebrew/homebrew-core@5fd233b#diff-84ad62c4f23782cf40fb7224d65ce62ab8950bb51bb3882f10bf397ae1606eaf but with the new tag and whatever the sha256 happens to be for the new release).

If you just want to use it from brew in the meantime, you should be able to do something like:

brew rm atomicparsley
brew install --HEAD atomicparseley

to have it pick up the master branch from this repo.

@wez
Copy link
Owner

wez commented Jan 15, 2021

I'm automating generation of my catalog from source .wavs,

Thanks for the link! I feel like I've heard ELECTROVIBE (XENON PLASMA MIX), or something similar, somewhere before but with female vocals over the top of it that I can't quite recall. I'm fairly sure that Spotify has inserted whatever that other track is into recommendations for me a few times.

@subatomicglue
Copy link
Author

subatomicglue commented Jan 15, 2021

Check out "Empty", it's a collaboration that I did with Tryad. Vavrek found my electrovibe track on the interwebs from the early 2000's, and sang some amazing haunting vocs on top. https://www.subatomicglue.com/disco/listen

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

No branches or pull requests

2 participants