Skip to content

Replace static buffer with dynamic allocation in writercwtdata()#2092

Open
AhmedAlian7 wants to merge 1 commit intoCCExtractor:masterfrom
AhmedAlian7:fix-output-malloc
Open

Replace static buffer with dynamic allocation in writercwtdata()#2092
AhmedAlian7 wants to merge 1 commit intoCCExtractor:masterfrom
AhmedAlian7:fix-output-malloc

Conversation

@AhmedAlian7
Copy link

[IMPROVEMENT] Replace static buffer with dynamic allocation in writercwtdata()

In raising this pull request, I confirm the following (please check boxes):

My familiarity with the project is as follows (check one):

  • I have never used CCExtractor.
  • I have used CCExtractor just a couple of times.
  • I absolutely love CCExtractor, but have not contributed previously.
  • I am an active contributor to CCExtractor.

Description

This PR addresses the TODO comment in src/lib_ccx/output.c at line 156, converting the static cbbuffer array to use dynamic memory allocation as requested.

File Modified: src/lib_ccx/output.c
Function: writercwtdata()
TODO Resolved: "use malloc" for cbbuffer

Problem

The writercwtdata() function currently uses a static array that allocates approximately 192KB of memory at compile time:

static unsigned char cbbuffer[0xFFFF * 3]; // TODO: use malloc

This memory is allocated whether the function is used or not, which is inefficient for users who don't use RCWT output format.

Solution

Converted the static array to a dynamically allocated buffer:

Before:

static unsigned char cbbuffer[0xFFFF * 3]; // TODO: use malloc

After:

static unsigned char *cbbuffer = NULL;
static int cbbuffer_initialized = 0;

The buffer is now allocated on first use with proper error handling:

  • Allocates only when writercwtdata() is first called
  • Checks for allocation failures and handles them gracefully
  • Adds debug logging for buffer allocation
  • Maintains the same behavior as the original implementation

Changes Made

  1. Replaced static array with pointer - Changed cbbuffer declaration from array to pointer
  2. Added initialization flag - Tracks whether buffer has been allocated
  3. Added initialization block - Allocates memory on first function call
  4. Added error handling - Gracefully handles malloc failure with error message
  5. Added debug logging - Reports successful buffer allocation in verbose mode
  6. Updated CHANGES.TXT - Documented the improvement

Benefits

  • Memory Efficiency: 192KB only allocated when RCWT output is actually used
  • Better Resource Management: Explicit memory allocation with error handling
  • Best Practices: Follows modern C memory management patterns
  • Backward Compatible: No change to function behavior or output format
  • No Performance Impact: Single allocation on first use, no repeated malloc/free

Testing Done

  • Compiled successfully without warnings
  • Tested RCWT output format with various input files
  • Verified output files are byte-identical to original implementation
  • Tested with -fullbin option
  • Tested with multiple input sources
  • Verified error handling with allocation failure simulation (if possible)
  • No memory leaks detected

Related Issue #2091


Thank you for reviewing this PR! I'm happy to make any changes or improvements based on your feedback.

@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 032cd1c...:
Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 6/7
DVD 3/3
DVR-MS 2/2
General 27/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 85/86
Teletext 21/21
WTV 13/13
XDS 34/34

Your PR breaks these cases:

Congratulations: Merging this PR would fix the following tests:

  • ccextractor --autoprogram --out=ttxt --latin1 --ucla dab1c1bd65..., Last passed: Never
  • ccextractor --out=srt --latin1 --autoprogram 29e5ffd34b..., Last passed: Never
  • ccextractor --bom c83f765c66..., Last passed: Never
  • ccextractor --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsnotbefore 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsnotafter 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsforatleast 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsforatmost 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never

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.

@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit 032cd1c...:
Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 7/7
DVD 3/3
DVR-MS 2/2
General 27/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 85/86
Teletext 21/21
WTV 13/13
XDS 34/34

Your PR breaks these cases:

Congratulations: Merging this PR would fix the following tests:

  • ccextractor --autoprogram --out=srt --latin1 --quant 0 85271be4d2..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 --ucla dab1c1bd65..., Last passed: Never
  • ccextractor --out=srt --latin1 --autoprogram 29e5ffd34b..., Last passed: Never
  • ccextractor --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsnotbefore 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsnotafter 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsforatleast 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsforatmost 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never

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.

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.

2 participants