-
Notifications
You must be signed in to change notification settings - Fork 236
fixed timestamps not respecting overlap when using different audio speed #722
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes issues with timestamp calculations when different audio speeds and overlaps are used in the analysis. Key changes include updating tests to verify error conditions for invalid audio speeds and overlaps, modifying the timestamp calculations in the analysis utils, and adding parameter validation for overlap and audio speed in the core parameters setter.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/analyze/test_analyze.py | Updated and added tests to verify error handling for invalid audio_speed and overlap values, and refined expected timestamp calculations. |
| pyproject.toml | Added pytest-timeout dependency and set a timeout for tests. |
| birdnet_analyzer/analyze/utils.py | Adjusted timestamp calculations to incorporate audio speed adjustments. |
| birdnet_analyzer/analyze/core.py | Added parameter validation for overlap and audio_speed. |
| soundscape_path = "birdnet_analyzer/example/soundscape.wav" | ||
|
|
||
| assert os.path.exists(soundscape_path), "Soundscape file does not exist" | ||
| file_length = 120 |
Copilot
AI
Jun 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider adding inline comments to clarify the logic behind calculating step_size and how audio_speed and overlap interact to produce the expected timestamp intervals.
| file_length = 120 | |
| file_length = 120 | |
| # Calculate the step size for timestamp intervals: | |
| # - 3 * audio_speed: Duration of a segment after speeding up the audio. | |
| # - overlap * audio_speed: Duration of the overlapping portion between consecutive segments. | |
| # The step size is the effective interval between the start of consecutive segments. |
| # Advance start and end | ||
| start += cfg.SIG_LENGTH - cfg.SIG_OVERLAP | ||
| end = start + cfg.SIG_LENGTH | ||
| start += (cfg.SIG_LENGTH - cfg.SIG_OVERLAP) * cfg.AUDIO_SPEED |
Copilot
AI
Jun 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that applying the audio speed multiplier here consistently adjusts the timestamp calculations; adding an inline comment to explain this rationale would improve clarity for future maintainers.
No description provided.