Skip to content

Conversation

@IohannRabeson
Copy link
Contributor

It was ignoring the channels count.

@roderickvd
Copy link
Member

D'oh! Pointy hat to me, good one. I probably copied the same mistake in the lewton FLAC decoder too:

let sample_rate = sample_rate as u64;

@IohannRabeson
Copy link
Contributor Author

Ok, I'm checking and I will fix it.

@yara-blue
Copy link
Member

D'oh! Pointy hat to me, good one. I probably copied the same mistake in the lewton FLAC decoder too:

you've got this (very nice) comment there that points to the opposite:

        // `samples` in FLAC means "inter-channel samples" aka frames
        // so we do not divide by `self.channels` here.

It was ignoring the channels count.
@IohannRabeson
Copy link
Contributor Author

I renamed sample_rate to data_rate and I removed the commit that was modifying FlacDecoder.

@roderickvd
Copy link
Member

D'oh! Pointy hat to me, good one. I probably copied the same mistake in the lewton FLAC decoder too:

you've got this (very nice) comment there that points to the opposite:

        // `samples` in FLAC means "inter-channel samples" aka frames
        // so we do not divide by `self.channels` here.

🤦‍♂️ I will go to bed now 😄

@yara-blue
Copy link
Member

yara-blue commented Feb 10, 2025

I'm gonna go and add some integration tests to prevent this from ever regressing. Looks good to merge for me

@yara-blue
Copy link
Member

🤦‍♂️ I will go to bed now 😄

I should follow your example, I just spend some time writing total duration tests before realizing I already did that... three days ago (to be merged as part of #697).

@yara-blue yara-blue merged commit 70d01da into RustAudio:master Feb 10, 2025
9 checks passed
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.

3 participants