[FIX] Remove unconditional VBI_DEBUG define — add CMake option for opt-in debug builds#2168
[FIX] Remove unconditional VBI_DEBUG define — add CMake option for opt-in debug builds#2168Varadraj75 wants to merge 3 commits intoCCExtractor:masterfrom
Conversation
VBI_DEBUG was hardcoded as always-on in ccx_decoders_vbi.h, causing debug-only struct fields (debug_file_name, vbi_debug_dump) and code paths to be compiled into every production build. Remove the unconditional #define VBI_DEBUG from the header and add a CMake option -DVBI_DEBUG=ON so developers can opt in explicitly, consistent with how other debug flags (WTV_DEBUG, DEBUG_SAVE_TS_PACKETS) work in the codebase. Fixes CCExtractor#2167
There was a problem hiding this comment.
Pull request overview
This PR fixes #2167 by removing an always-on VBI_DEBUG preprocessor define from the VBI decoder header and replacing it with an opt-in CMake build option, preventing debug-only VBI fields/paths from being compiled into default production builds.
Changes:
- Removed the unconditional
#define VBI_DEBUGfromsrc/lib_ccx/ccx_decoders_vbi.h. - Added a
VBI_DEBUGCMake option intended to conditionally defineVBI_DEBUGat compile time. - Documented the change in
docs/CHANGES.TXT.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/lib_ccx/ccx_decoders_vbi.h |
Removes unconditional debug macro to avoid always compiling debug-only fields/code. |
src/CMakeLists.txt |
Adds VBI_DEBUG build option and attempts to define VBI_DEBUG when enabled. |
docs/CHANGES.TXT |
Changelog entry describing the fix and the new opt-in build flag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The if(VBI_DEBUG) block was placed after add_subdirectory(lib_ccx), meaning add_definitions(-DVBI_DEBUG) would not reach the library code. Moved the block before add_subdirectory(lib_ccx) so the flag correctly affects ccx_decoders_vbi.c and related files. Addresses Copilot review comment on CCExtractor#2168.
MSVC (C2016) requires structs to have at least one member. When VBI_DEBUG is off (default), ccx_decoder_vbi_cfg was empty causing Windows build failure. Add a reserved int member under #ifndef VBI_DEBUG to satisfy the C standard requirement on MSVC. Fixes Windows CI failure on CCExtractor#2168.
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit f377be9...:
Congratulations: Merging this PR would fix the following tests:
All tests passed completely. Check the result page for more info. |
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit f377be9...:
Your PR breaks these cases:
It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you). Check the result page for more info. |
|
The Windows CI test failures are pre-existing and unrelated to this PR. All failing tests show The previous Windows build failure (C2016 empty struct) has been fixed Linux CI passes all tests cleanly. |
…o fprintf(stderr) - Remove #define DEBUG_OUT 0 from networking.c and replace all #if DEBUG_OUT guards with #ifdef NETWORKING_DEBUG, consistent with how other debug flags work in the codebase (see CCExtractor#2168) - Add CMake option -DNETWORKING_DEBUG=ON for opt-in debug builds - Replace 3 error messages using printf() with fprintf(stderr): 'Can't send BIN header' 'Can't send BIN data' 'Unable to send data' Error output should go to stderr so it is visible when stdout is redirected (e.g. ccextractor input.ts > output.srt) Fixes CCExtractor#2174
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
Summary
Fixes #2167 —
VBI_DEBUGwas unconditionally defined inccx_decoders_vbi.h,causing debug-only struct fields and code paths to always be compiled into
production builds.
Root Cause
This caused
debug_file_nameandvbi_debug_dumpfields to be present inevery build of
ccx_decoder_vbi_cfgandccx_decoder_vbi_ctx, regardlessof whether debug output was needed.
Fix
Removed the unconditional
#define VBI_DEBUGfrom the header and added aproper CMake option consistent with how other debug flags work in the project:
Developers who need VBI debug output can now opt in explicitly:
Comparison
Other debug flags in the codebase (
WTV_DEBUG,DEBUG_SAVE_TS_PACKETS) arenever unconditionally defined — they require explicit opt-in. This fix brings
VBI_DEBUGin line with that pattern.Testing
Built and verified locally on macOS with both
VBI_DEBUG=OFF(default)and
VBI_DEBUG=ON. All existing CI tests pass.Fixes #2167