search

yotarok / flacenc-rs

FLAC encoder written in Rust
Apache License 2.0
22 stars 3 forks source link

Change to store sampling rates in frame headers. #198

Closed yotarok closed 3 months ago

yotarok commented 3 months ago

This closes #195.

dtzxporter commented 3 months ago

I tested this, some flac files are now working!

Oddly enough, now, chrome won't even play the beginning of the broken sample. QT Still plays silence. For the fixed flac files, chrome/QT play them fully as expected.

I'm still running into an issue with the original sample though, re-uploaded using this branch:

Archive.zip

dtzxporter commented 3 months ago

With flac --test:

image

dtzxporter commented 3 months ago

Looks like a problem with 'custom' sample rates, the ones that work have one of the stock ones.

yotarok commented 3 months ago

Thanks for testing. I found a mistake in the previous commit 009f63d. The extra sampling-rate bits (only used when sampling rates are atypical) are written in a wrong bitstream. The integration test is improved to cover this.

dtzxporter commented 3 months ago

Awesome. This fixes the issue playing the encoded flacs in QT/Chrome perfectly.

It does not solve the issue with the last block's samples being included in the output of any decoder, which also causes checksums to be incorrect in most tools.

With libflac, it will decode fine, but the resulting wav will have a bunch of extra empty samples at the end (however, it truncates the data size itself, it's just writing bad data...)

With ffmpeg/fq (a cool diagnostic tool) they calculate the checksum wrong (because of the extra samples ~3000 in the example I gave above) and spit out the wrong sample count as well.

FQ output: image

yotarok commented 3 months ago

It does not solve the issue with the last block's samples being included in the output of any decoder, which also causes checksums to be incorrect in most tools.

I'm aware of this issue. I was thinking that it's not problematic to have extra zero-samples on the end of the file, as it is truncated in flac reference decoder. But, since ffmpeg warns about this, it might be an actual issue that needs to be fixed.

I was reluctant to implement the last-frame truncation (as in flac reference encoder) because flacenc-rs states that it currently only supports encoding with "fixed" block-size, and the last-frame truncation violates this statement. I'm now thinking implementing it, but anyway this should be in another PR.