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

Testing file encoded with FLAC 1.3.2 fails when tested with flac.exe 1.4.3 #653

Closed
TT10 opened this issue Oct 29, 2023 · 10 comments · Fixed by #738
Closed

Testing file encoded with FLAC 1.3.2 fails when tested with flac.exe 1.4.3 #653

TT10 opened this issue Oct 29, 2023 · 10 comments · Fixed by #738

Comments

@TT10
Copy link

TT10 commented Oct 29, 2023

I was batch-testing thousands of FLAC files using flac.exe 1.4.3 and some were reported as corrupted.

After some testing with 1.3.2 (which is the oldest version currently on my computer) and 1.4.3 (latest...), I noticed the follow results:

  • File encoded with flac.exe 1.4.3, tested with 1.4.3 -> OK
  • File encoded with flac.exe 1.4.3, tested with 1.3.2 -> OK
  • File encoded with flac.exe 1.3.2, tested with 1.3.2 -> OK
  • File encoded with flac.exe 1.3.2, tested with 1.4.3 -> Fails: "state = FLAC__STREAM_DECODER_END_OF_STREAM"

Please find some test files here attached.
20231029-FLAC_testing_possible_bug-Test_files_exept_FLAC_executable_without_wav.zip
This sample .zip does NOT include:

  • flac.exe 1.3.2 and 1.4.3, since I guess it would make cleaner testing on your side if you redownload them from source.
  • My original test .wav file (because GitHub doesn't allow files larger than 25 MB nor .rar nor .7z archives.)

This .zip does include:

  • Two .flac files, "Sound1-1.3.2.flac", encoded with flac.exe 1.3.2 and "Sound1-1.4.3.flac", encoded with flac.exe 1.4.3
  • Batch files containing the command lines I used to encode and test.

My first thought is that this may be a bug in 1.4.3 testing functions. I am aware that this may be a result a bug from 1.3.2 that would have got corrected later, but I don't follow FLAC's bug tracking so I would like to know... And if it is a 1.4.3 bug, I hope it will get corrected.

@TT10
Copy link
Author

TT10 commented Oct 29, 2023

Sorry, just after posting, I realise that this is the same issue as #487

@ktmf01
Copy link
Collaborator

ktmf01 commented Nov 10, 2023

Could you check again, both files test 'ok' at my end.

@TT10
Copy link
Author

TT10 commented Nov 12, 2023

You're right, there was a mistake in my .bat file ( named: Test_1.3.2_with_flac_exe_1.4.3.bat ).

So the issue doesn't appear with these test files that only went through FLAC.exe.

But the weird thing is that this matched exactly my observations on a lot of other files from my listening and archive library (which kind of hid the mistake in the .bat file while testing).

Differences between my archive files and test files is that my archive files have gone through these:

  • Encoded by EAC (Excat Audio Copy), or CUERipper (which I used for some times some time ago and I don't remember exactly when I switched). (both use some version of flac.exe and include some metadate)
  • Almost certainly: Tagged using MP3tag
  • Less likely: Tagged in Foobar2k (v1.x)
  • Possibly: Had thumbnail removed using metaflac.exe

Would it help if I send one of these files ? (I didn't in the first place as I wanted to keep it to flac.exe in my first tests, and they contain copyrighted music ... yet, they're files that yield "healthy" when tested with flac 1.3.2 and "corrupted" when tested with flac 1.4.3.

@H2Swine
Copy link
Contributor

H2Swine commented Nov 13, 2023

EAC has a checkbox to add ID3, and if you selected that, you would likely trigger a #487 situation.

@TT10
Copy link
Author

TT10 commented Nov 15, 2023

Thank you, this is right. I compared using files that pass test and files that don't. Those that pass test only have "FLAC" metatags, while those that don't have "FLAC ID3v2 ID3v1".

Is there a way I can remove "ID3v2 ID3v1" with removing "FLAC" meta data?

Edit: I found how, the topic apparently isn't new (I simply notice it now due to switching to FLAC 1.4.3.
This thread ( https://hydrogenaud.io/index.php/topic,79525.0.html ) explains: "MP3Tag does exactly what you want. Just load your files, select them, remove the tags, then select undo." Which I just tested and works.

I also stumbled upon a bash script... which I'm not sure how to run but I could find a way.

(I expect most of my collection was made with CUERipper and not EAC ... I don't remember exactly when I switched ... so I don't think I should look for a way to automate this.)

@TT10
Copy link
Author

TT10 commented Nov 15, 2023

Just thought about this right now: I'm still thinking that this is a "bug"... As I read in 1.4.2 changelog, I think, that -t now warns if ID3 tags are present. There is indeed a warning. But if we get a warning for one thing, the testing of the audio part should still pass. Also because, if I simply remove the ID3 tags (as mentioned in my previous reply), the file then passes testing.

@JohnLGalt
Copy link

Might look into MetaBrainz' Picard. I've stopped using any other tagging solution, as it just works and works correctly.

However, I'd also need to run your verification analysis on my FLAC files made via EAC, both prior to tagging as well as after tagging (and renaming and moving to my defined folder structure).

I'll grab your test file from the OP and see what comes out on my end.

@ktmf01
Copy link
Collaborator

ktmf01 commented Nov 15, 2023

Just thought about this right now: I'm still thinking that this is a "bug"...

Yes, that is why #487 is still open

As I read in 1.4.2 changelog, I think, that -t now warns if ID3 tags are present.

It says a warning is added for ID3v2. But it seems your files have ID3v1 tags, which are currently not detected. FLAC versions prior to 1.4.0 would just stop decoding if the number of samples specified in the streaminfo block was reached, but this resulted in a number of problems. In FLAC 1.4.0 and above, the FLAC decoder keeps decoding. It then stumbles on an ID3v1 block because it doesn't recognize it and tries to decode it as if it were audio.

@TT10
Copy link
Author

TT10 commented Nov 15, 2023

Might look into MetaBrainz' Picard. I've stopped using any other tagging solution, as it just works and works correctly.

I may take a look, thank you. Though usually I've been satisfied with first auto-tagging with CUERipper or EAC and then completing and harmonising at my taste with MP3tag. (These last days I've trying to remember why I switched back from CUERipper to EAC...)

However, I'd also need to run your verification analysis on my FLAC files made via EAC, both prior to tagging as well as after tagging (and renaming and moving to my defined folder structure). I'll grab your test file from the OP and see what comes out on my end.

The files in the OP won't help you as they are not EAC files. They are generated sounds encoded directly through flac.exe.
I may try ripping some more test files from some copyright-free music CD, when I find time.

It says a warning is added for ID3v2. But it seems your files have ID3v1 (...)

You are probably right. As I just replied to JohnLGalt's reply, I'll try to do some tests on this and rip some more test files from some copyright-free music CD, when I find time.

@JohnLGalt
Copy link

The files in the OP won't help you as they are not EAC files. They are generated sounds encoded directly through flac.exe. I may try ripping some more test files from some copyright-free music CD, when I find time.

No need (for me) - I have tons of files (hundreds of GB worth) that I can test myself, plus I have both EAC and CUERipper, so I can test with both, and see if it is worth going back to CUERipper or stick with EAC. I also have tons of discs I can RIP or reRIP as needed.

I just wanted your scripts themselves for testing, not the files you tested with, in order to keep the testing consistent.

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 a pull request may close this issue.

4 participants