Repository navigation
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to SoundFlow repository! We appreciate you taking the time to contribute.
We're excited to review your pull request and look forward to collaborating with you. Please let us know if you have any questions or need any assistance.
Thank you for your contribution!
PR Summary by QodoFix MP3 duration when metadata exceeds decoder length
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can choose which labels appear on a finding, and whether they show icons or text |
 [Medium impact] The changes reach MP3 duration estimates and stream-backed audio lengths. The PR appears safe to merge, with a non-blocking request to check in the regression tests. Findings
Fix with agent prompt### Issue 1
Src/Providers/StreamDataProvider.cs:135-145
The checks described in the PR live outside the repository, so they cannot catch future changes that restore the wrong duration or seek limit. Please check in tests for the MPEG-version bitrate rows, a positive decoder length smaller than metadata, and an unknown decoder length with different source and output formats. Include invalid durations and the `int.MaxValue` boundary.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryFixes MP3 bitrate lookup and makes
Reviews (1) · Last reviewed commit: "fix: preserve decoder length and correct..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| var decoderLength = _decoder.Length; | ||
|
|
||
| int metadataLength; | ||
| if (FormatInfo != null | ||
| && FormatInfo.Duration > TimeSpan.Zero | ||
| && FormatInfo.SampleRate > 0 | ||
| && FormatInfo.ChannelCount > 0) | ||
| { | ||
| var metadataLengthSamples = FormatInfo.Duration.TotalSeconds | ||
| * FormatInfo.SampleRate | ||
| * FormatInfo.ChannelCount; | ||
|
|
||
| if (metadataLengthSamples is >= 0 and <= int.MaxValue) | ||
| metadataLength = (int)metadataLengthSamples; | ||
| else | ||
| metadataLength = 0; | ||
| } | ||
| else | ||
| { | ||
| metadataLength = 0; | ||
| } | ||
| if (decoderLength > 0) | ||
| return decoderLength; | ||
|
|
||
| return decoderLength switch | ||
| { | ||
| // Prefer the larger value when both are available. See the XML doc above for the rationale. | ||
| // Special case: if one of them is 0 (unknown), use the other. | ||
| > 0 when metadataLength > 0 => Math.Max(decoderLength, metadataLength), | ||
| > 0 => decoderLength, | ||
| _ => metadataLength | ||
| }; | ||
| if (FormatInfo == null || FormatInfo.Duration <= TimeSpan.Zero | ||
| || _decoder.SampleRate <= 0 || _decoder.Channels <= 0) | ||
| return 0; | ||
|
|
||
| var metadataLengthSamples = FormatInfo.Duration.TotalSeconds | ||
| * _decoder.SampleRate | ||
| * _decoder.Channels; |
There was a problem hiding this comment.
The checks described in the PR live outside the repository, so they cannot catch future changes that restore the wrong duration or seek limit. Please check in tests for the MPEG-version bitrate rows, a positive decoder length smaller than metadata, and an unknown decoder length with different source and output formats. Include invalid durations and the int.MaxValue boundary.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Src/Providers/StreamDataProvider.cs
Line: 135-145
Comment:
**Regression checks aren't checked in**
The checks described in the PR live outside the repository, so they cannot catch future changes that restore the wrong duration or seek limit. Please check in tests for the MPEG-version bitrate rows, a positive decoder length smaller than metadata, and an unknown decoder length with different source and output formats. Include invalid durations and the `int.MaxValue` boundary.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Fix MP3 duration regression when metadata overestimates the decoder length
A 192 kb/s MPEG-1 MP3 without a Xing/Info header is read as 112 kb/s, and the 1.4.2 provider chooses that inflated metadata estimate over the decoder's known length. A file that decodes to 10.032 seconds consequently reports 17.198 seconds and accepts seeks past its actual end.
Select the MPEG-version bitrate row for Layer III, preserving the single-row lookups for the other layers. Prefer a positive decoder length and use metadata only when the decoder length is unknown. Convert that fallback into output samples using the decoder's sample rate and channel count, retaining the range checks. The example above now reports 10.032 seconds and rejects a seek to 12 seconds.
Closes #118.
Validation
expected 192000, got 112000and exits successfully with this patch.The minimal reproduction and file-generation command are included in #118. Build the core library from the repository root with: