Make ps_start_stream() succeed when noise removal is disabled - #493
Open
lenzo-ka wants to merge 2 commits into
Open
Make ps_start_stream() succeed when noise removal is disabled#493lenzo-ka wants to merge 2 commits into
lenzo-ka wants to merge 2 commits into
Conversation
No functional change. The definition was private to fe_noise.c; internal consumers, including tests, can now compile against the real layout instead of duplicating it.
With the default remove_noise=no the front end allocates no noise statistics, and ps_start_stream() returned -1 on a successfully initialized decoder. Callers that treat a negative return as failure rejected a valid decoder in the default configuration, and the documented contract states no such precondition. fe_reset_noisestats() is already a no-op on a null pointer, so call it unconditionally and return 0. Document the no-op case in the header. Behavior with noise removal enabled is unchanged. test_start_stream covers both settings: it asserts a zero return with noise removal disabled, and that the statistics are actually reset when it is enabled. The disabled case fails against the previous implementation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ps_start_stream()returned -1 whenever noise removal was disabled, which is the shipped default (-remove_noise no). The front end allocates noise statistics only when noise removal is enabled, andps_start_stream()treated their absence as an error, so a caller of this documented compatibility function got a failure return from a successfully initialized decoder. The header states no such precondition, andfe_reset_noisestats()is already a no-op on a null pointer.The fix calls
fe_reset_noisestats()unconditionally and returns 0; when there are no statistics to reset the call is a successful no-op, now stated in the header documentation. Behavior with noise removal enabled is unchanged.A first commit moves the
noise_stats_sdefinition fromfe_noise.cinto the internal headerfe_noise.h, with no functional change, so internal consumers can compile against the real layout instead of duplicating it.The new unit test
test_start_streaminitializes a real decoder both ways: with noise removal disabled it asserts a zero return, and with it enabled it asserts the statistics are actually reset. The disabled case fails against the previous implementation.Fixes #496
Tested on macOS (Apple clang) and Linux x86-64 (GCC 13.3); full unit and regression suite passes on both.