FLAC files with artwork embedded by ATL fails flac -t

GOATS2K commented 1 month ago

The problem

The FLAC integrity test fails on files with artwork that is embedded by ATL.

Environment

ATL version (or git revision) that exhibits the issue: 5.25.0
Last ATL version that did not exhibit the issue (if applicable): N/A
OS/version used to run ATL: macOS Sonoma 14.5

Details

You can reproduce this by first finding a FLAC file that passes flac -t file.flac.

➜ flac -t 01\ -\ Submorphics\ \&\ Lenzman\ -\ Echoes\ of\ November.flac

flac 1.4.3
Copyright (C) 2000-2009  Josh Coalson, 2011-2023  Xiph.Org Foundation
flac comes with ABSOLUTELY NO WARRANTY.  This is free software, and you are
welcome to redistribute it under certain conditions.  Type `flac' for details.

01 - Submorphics & Lenzman - Echoes of November.flac: ok

Then using ATL to embed artwork like this:

public async Task WriteArtwork(string filePath, Stream artworkStream)
{
    var memoryStream = new MemoryStream();
    await artworkStream.CopyToAsync(memoryStream);

    var track = new ATL.Track(filePath);
    var pictureEmbed = PictureInfo.fromBinaryData(memoryStream.ToArray(),
        PictureInfo.PIC_TYPE.Front,
        MetaDataIOFactory.TagType.RECOMMENDED);

    track.EmbeddedPictures.Add(pictureEmbed);
    track.Save();
}

Observe that flac -t now fails with the following error:

01 - Submorphics & Lenzman - Echoes of November.flac: *** Got error code 0:FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC

01 - Submorphics & Lenzman - Echoes of November.flac: ERROR while decoding data
                                          state = FLAC__STREAM_DECODER_ABORTED

Zeugma440 commented 1 month ago

Roger that. I'm gonna take care of it in a few weeks as I'm currently on holidays 😎

GOATS2K commented 1 month ago

Enjoy your holiday!

Zeugma440 commented 1 month ago

Hi there~

Maybe I'm out of luck, but I just tried on two very different FLAC files, and suceeded the check after adding the cover.

Could you please send me that FLAC file you're using through a 3rd party file hosting service? My address is (my Github username) [at] (not-cold)mail [dot] com

GOATS2K commented 1 month ago

Did you use the latest flac version? I've found the strictness to vary a bit between versions.

On Fri, 2 Aug 2024 at 21:52, Zeugma440 @.***> wrote:

Hi there~

Maybe I'm out of luck, but I just tried on two very different FLAC files, and suceeded the check after adding the cover.

Could you please send me that FLAC file you're using through a 3rd party file hosting service? My address is (my Github username) [at] (not-cold)mail [dot] com

— Reply to this email directly, view it on GitHub https://github.com/Zeugma440/atldotnet/issues/272#issuecomment-2266130728, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANI5XUV7F5KXROJ7HPTSQ6DZPPWPHAVCNFSM6AAAAABKY6SCM6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRWGEZTANZSHA . You are receiving this because you authored the thread.Message ID: @.***>

Zeugma440 commented 1 month ago

I used the flac command line exe v1.4.3

GOATS2K commented 1 month ago

Okay, I'll send you the files!

On Sat, 3 Aug 2024 at 10:28, Zeugma440 @.***> wrote:

I used the flac command line exe v1.4.3

— Reply to this email directly, view it on GitHub https://github.com/Zeugma440/atldotnet/issues/272#issuecomment-2266652753, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANI5XUVWUFV2RSU4BZCLDYLZPSPEFAVCNFSM6AAAAABKY6SCM6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRWGY2TENZVGM . You are receiving this because you authored the thread.Message ID: @.***>

GOATS2K commented 1 month ago

Check your email! :)

Zeugma440 commented 1 month ago

Thanks for the files (let's call them OK file and KO file)!

I can reproduce flac -t detecting the KO file as corrupted.
I cannot reproduce your issue by embedding a cover in the OK file once.
I can reproduce your issue by embedding a cover in the OK file twice 😅.

I'll fix that very soon and let you know~

PS : I noticed you're calling Track.Save() inside an async method. Did you see there's a Track.SaveAsync() method?

GOATS2K commented 1 month ago

Yes, however I recalled that SaveAsync was slower than sync Save - so I use the sync Save method instead.

That sounds about right, I store the general metadata first and then the artwork if it is present, so that explains why saving it twice reproduces the issue. Thanks for figuring it out!

On Sun, 4 Aug 2024 at 12:00, Zeugma440 @.***> wrote:

Thanks for the files (let's call them OK file and KO file)!

I can reproduce flac -t detecting the KO file as corrupted.

I cannot reproduce your issue by embedding a cover in the OK file once.

I can reproduce your issue by embedding a cover in the OK file twice 😅.

I'll fix that very soon and let you know~

PS : I noticed you're calling Track.Save() inside an async method. Did you see there's a Track.SaveAsync() method?

— Reply to this email directly, view it on GitHub https://github.com/Zeugma440/atldotnet/issues/272#issuecomment-2267500227, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANI5XUV2GQH3CQGJYQICDTDZPYCWLAVCNFSM6AAAAABKY6SCM6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRXGUYDAMRSG4 . You are receiving this because you authored the thread.Message ID: @.***>

Zeugma440 commented 1 month ago

Fix is done; however I have to tell you that your code will add multiple versions of the same picture to your file, as you call track.EmbeddedPictures.Add(pictureEmbed); on top of a file that might already have that image embedded, and FLAC specs allow for multiple front covers inside the same file.

I recalled that SaveAsync was slower than sync Save

That's for sure. However, as you're working on FLAC files, did you customize Settings.FileBufferSize to something bigger than the default, say 2MB? That one setting plays a big role in performance.

GOATS2K commented 1 month ago

Yep, changing buffer sizes made a huge difference in performance. Looking forward to trying out your fix soon! :)

On Sun, 4 Aug 2024 at 16:07, Zeugma440 @.***> wrote:

Fix is done; however I have to tell you that your code will add multiple versions of the same picture to your file, as you call track.EmbeddedPictures.Add(pictureEmbed); on top of a file that might already have that image embedded, and FLAC specs allow for multiple front covers inside the same file.

I recalled that SaveAsync was slower than sync Save

That's for sure. However, as you're working on FLAC files, did you customize Settings.FileBufferSize to something bigger than the default, say 2MB? That one setting plays a big role in performance.

— Reply to this email directly, view it on GitHub https://github.com/Zeugma440/atldotnet/issues/272#issuecomment-2267574923, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANI5XURV3FXQZQLY2VXDXWDZPY7RZAVCNFSM6AAAAABKY6SCM6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRXGU3TIOJSGM . You are receiving this because you authored the thread.Message ID: @.***>

Zeugma440 commented 1 month ago

You can try it with today's v5.26.

Please close the issue if it works on your side~

GOATS2K commented 1 month ago

Yep, that works! Thank you for the fix!

Zeugma440 / atldotnet