[FIX] Restore free_sub_track scope and keep blockaddition cleanup by Shiv0087 · Pull Request #2270 · CCExtractor/ccextractor

Shiv0087 · 2026-04-22T09:37:23Z

Summary

This PR fixes the free_sub_track() cleanup logic after the previous patch introduced a scope/indentation issue.

What changed

Restored the missing closing brace in free_sub_track().
Kept cleanup for WebVTT block additions:
- free sentence->blockaddition->cue_settings_list (when non-NULL)
- free sentence->blockaddition
Ensured free(track->sentences) and free(track) are executed after the sentence loop.
Restored consistent tab indentation to match the file style.

Why

The previous diff unintentionally broke loop scope, which moved track-level frees into the wrong context.
This patch preserves the intended memory-leak fix while restoring correct control flow and style.

Validation

Verified function logic and ownership cleanup paths by code inspection.
Please run CI/build checks in repository workflow.

Shiv0087 · 2026-04-22T09:37:38Z

Supersedes #2249 with the requested scope/brace fix, tab indentation cleanup, and retained blockaddition memory cleanup.

Copilot

Pull request overview

Fixes free_sub_track() control-flow/indentation so per-sentence cleanup happens inside the loop while track-level cleanup (free(track->sentences) / free(track)) happens after the loop, and retains the WebVTT blockaddition cleanup added previously.

Changes:

Restores correct brace scope in free_sub_track() so track-level frees are not executed per-sentence.
Adds cleanup for sentence->blockaddition and its backing buffer (cue_settings_list) when present.
Normalizes whitespace/blank lines for readability and consistency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+			/* cue_settings_list is the base of the message buffer;
+			 * cue_identifier and comment are pointers into it */
+			if (sentence->blockaddition->cue_settings_list != NULL)
+				free(sentence->blockaddition->cue_settings_list);


Copilot

Pull request overview

This PR corrects the control-flow/scope of free_sub_track() in the Matroska subtitle parser and attempts to preserve the WebVTT BlockAddition cleanup that was added in a prior patch.

Changes:

Restores the intended loop scope in free_sub_track() so track-level frees happen after the sentence loop.
Adds cleanup for sentence->blockaddition and its associated buffer pointer (cue_settings_list) when present.
Adjusts formatting/whitespace to match existing file style.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+			/* cue_settings_list is the base of the message buffer;
+			 * cue_identifier and comment are pointers into it */
+			if (sentence->blockaddition->cue_settings_list != NULL)
+				free(sentence->blockaddition->cue_settings_list);
+


cfsmp3

The scope/indentation fix is correct — that was the main problem with #2249. But there's a memory leak edge case that Copilot caught and is real.

The issue

parse_segment_cluster_block_group_block_additions() allocates message via read_bytes_signed(). Then cue_settings_list, cue_identifier, and comment are set as pointers INTO that message buffer. But cue_settings_list is only assigned when the first line is non-empty (item_size > 0):

case 0:
    if (item_size > 0)        // ← only if non-empty
    {
        newBA->cue_settings_list = message + lastIndex;
        ...
    }

If cue settings are absent (empty first line), cue_settings_list stays NULL. Your cleanup code then skips the free:

if (sentence->blockaddition->cue_settings_list != NULL)
    free(sentence->blockaddition->cue_settings_list);

Result: message buffer leaked.

Fix

Store the message pointer directly in struct block_addition:

// In matroska.h, add to struct block_addition:
char *message_buf;   // owning pointer to the backing buffer

// In parse_segment_cluster_block_group_block_additions():
newBA->message_buf = message;

// In free_sub_track():
free(sentence->blockaddition->message_buf);  // always frees the backing buffer
free(sentence->blockaddition);

This way the owning pointer is always available regardless of whether cue_settings_list was set.

Also: add braces to if (sentence->blockaddition->cue_settings_list != NULL) per our code style.

Shiv0087 · 2026-04-24T10:40:43Z

Addressed: added message_buf as owning pointer in struct block_addition, set it in parse_segment_cluster_block_group_block_additions(), and free it in free_sub_track(). This fixes the empty cue-settings leak edge case.

ccextractor-bot · 2026-04-24T11:29:57Z

CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 2feb09a...:

Report Name	Tests Passed
Broken	8/13
CEA-708	1/14
DVB	2/7
DVD	3/3
DVR-MS	2/2
General	22/27
Hardsubx	1/1
Hauppage	3/3
MP4	3/3
NoCC	10/10
Options	73/86
Teletext	20/21
WTV	12/13
XDS	29/34

Your PR breaks these cases:

ccextractor --autoprogram --out=ttxt --latin1 --ucla --xds 8e8229b88b...
ccextractor --out=srt --latin1 611b4a9235...
ccextractor --autoprogram --out=srt --latin1 --quant 0 85271be4d2...
ccextractor --autoprogram --out=ttxt --latin1 99e5eaafdc...
ccextractor --autoprogram --out=ttxt --latin1 --ucla 7aad20907e...
ccextractor --autoprogram --out=ttxt --latin1 --ucla dab1c1bd65...
ccextractor --autoprogram --out=ttxt --latin1 01509e4d27...
ccextractor --out=srt --latin1 --autoprogram 29e5ffd34b...
ccextractor --out=spupng c83f765c66...
ccextractor --dru c83f765c66...
ccextractor --startat 4 --endat 7 c83f765c66...
ccextractor --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
ccextractor --startcreditsnotbefore 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
ccextractor --startcreditsnotafter 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
ccextractor --startcreditsforatleast 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
ccextractor --startcreditsforatmost 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
ccextractor --out=srt --latin1 f23a544ba8...
ccextractor --autoprogram --out=srt --latin1 --ucla d037c7509e...
ccextractor --autoprogram --out=ttxt --xds --latin1 --ucla 85058ad37e...
ccextractor --autoprogram --out=srt --latin1 --ucla b22260d065...
ccextractor --autoprogram --out=srt --latin1 --ucla 7d3f25c32c...
ccextractor --autoprogram --out=ttxt --latin1 --ucla --xds 7f41299cc7...

NOTE: The following tests have been failing on the master branch as well as the PR:

ccextractor --out=srt --latin1 --autoprogram 73d9313d64..., Last passed:
Test 8738
ccextractor --out=ttxt --latin1 001dd8cdf7..., Last passed:
Test 8738
ccextractor --out=srt --latin1 4d4e938ef6..., Last passed:
Test 8738
ccextractor --service 1 --out=txt --no-bom --no-rollup ea83ff7bcb..., Last passed:
Test 8738
ccextractor --service 1 --out=txt f17524b53f..., Last passed:
Test 8738
ccextractor --service 1 --out=txt 80848c45f8..., Last passed:
Test 8738
ccextractor --service 1 --out=txt --no-bom --no-rollup b5d6aad89f..., Last passed:
Test 8738
ccextractor --service 1[EUC-KR] --out=txt --no-rollup b5d6aad89f..., Last passed:
Test 8738
ccextractor --service 1 --out=srt da904de35d..., Last passed:
Test 8738
ccextractor --service 1 --out=sami da904de35d..., Last passed:
Test 8738
ccextractor --service 1 --out=ttxt da904de35d..., Last passed:
Test 8926
ccextractor --service 1[EUC-KR] b5d6aad89f..., Last passed:
Test 8738
ccextractor --service 1[EUC-KR] --no-rollup b5d6aad89f..., Last passed:
Test 8738
ccextractor --service all da904de35d..., Last passed:
Test 8738
ccextractor --service all[EUC-KR] b5d6aad89f..., Last passed:
Test 8738
ccextractor --service 1,2[UTF-8],3[EUC-KR],54 --out=txt da904de35d..., Last passed:
Test 8738
ccextractor --autoprogram --out=srt --latin1 f1422b8bfe..., Last passed:
Test 9208
ccextractor --autoprogram --out=srt --latin1 d41b53b504..., Last passed:
Test 8738
ccextractor --stdout --quiet --no-fontcolor 79a51f3500..., Last passed:
Test 8738
ccextractor --stdout --quiet --no-fontcolor 767b546f96..., Last passed:
Test 8738
ccextractor --service 1 c83f765c66..., Last passed:
Test 8738
ccextractor --myth c83f765c66..., Last passed:
Test 8738
ccextractor --in=raw fb79021542..., Last passed:
Test 8738
ccextractor --mp4vidtrack 5df914ce77..., Last passed:
Test 8738
ccextractor --xmltv=3 --out=null 96efd279cf..., Last passed:
Test 8738
ccextractor --datapid 2310 --autoprogram --out=srt --latin1 e639e54550..., Last passed:
Test 8738

Congratulations: Merging this PR would fix the following tests:

ccextractor --autoprogram --out=ttxt --latin1 132d7df7e9..., Last passed: Never
ccextractor --autoprogram --out=srt --latin1 b22260d065..., 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 · 2026-04-24T11:49:22Z

CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit 2feb09a...:

Report Name	Tests Passed
Broken	8/13
CEA-708	1/14
DVB	3/7
DVD	3/3
DVR-MS	2/2
General	22/27
Hardsubx	1/1
Hauppage	3/3
MP4	3/3
NoCC	10/10
Options	74/86
Teletext	20/21
WTV	12/13
XDS	29/34

Your PR breaks these cases:

ccextractor --autoprogram --out=ttxt --latin1 --ucla --xds 8e8229b88b...
ccextractor --out=srt --latin1 611b4a9235...
ccextractor --autoprogram --out=ttxt --latin1 132d7df7e9...
ccextractor --autoprogram --out=ttxt --latin1 99e5eaafdc...
ccextractor --autoprogram --out=srt --latin1 b22260d065...
ccextractor --autoprogram --out=ttxt --latin1 --ucla 7aad20907e...
ccextractor --autoprogram --out=ttxt --latin1 01509e4d27...
ccextractor --dru c83f765c66...
ccextractor --startat 4 --endat 7 c83f765c66...
ccextractor --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
ccextractor --startcreditsnotbefore 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
ccextractor --startcreditsnotafter 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
ccextractor --startcreditsforatleast 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
ccextractor --startcreditsforatmost 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
ccextractor --out=srt --latin1 f23a544ba8...
ccextractor --autoprogram --out=srt --latin1 --ucla d037c7509e...
ccextractor --autoprogram --out=ttxt --xds --latin1 --ucla 85058ad37e...
ccextractor --autoprogram --out=srt --latin1 --ucla b22260d065...
ccextractor --autoprogram --out=srt --latin1 --ucla 7d3f25c32c...
ccextractor --autoprogram --out=ttxt --latin1 --ucla --xds 7f41299cc7...

NOTE: The following tests have been failing on the master branch as well as the PR:

ccextractor --out=srt --latin1 --autoprogram 73d9313d64..., Last passed:
Test 8611
ccextractor --out=ttxt --latin1 001dd8cdf7..., Last passed:
Test 8611
ccextractor --out=srt --latin1 4d4e938ef6..., Last passed:
Test 8611
ccextractor --service 1 --out=txt --no-bom --no-rollup ea83ff7bcb..., Last passed:
Test 8611
ccextractor --service 1 --out=txt f17524b53f..., Last passed:
Test 8611
ccextractor --service 1 --out=txt 80848c45f8..., Last passed:
Test 8611
ccextractor --service 1 --out=txt --no-bom --no-rollup b5d6aad89f..., Last passed:
Test 8611
ccextractor --service 1[EUC-KR] --out=txt --no-rollup b5d6aad89f..., Last passed:
Test 8611
ccextractor --service 1 --out=srt da904de35d..., Last passed:
Test 8611
ccextractor --service 1 --out=sami da904de35d..., Last passed:
Test 8611
ccextractor --service 1 --out=ttxt da904de35d..., Last passed:
Test 8943
ccextractor --service 1[EUC-KR] b5d6aad89f..., Last passed:
Test 8611
ccextractor --service 1[EUC-KR] --no-rollup b5d6aad89f..., Last passed:
Test 8611
ccextractor --service all da904de35d..., Last passed:
Test 8611
ccextractor --service all[EUC-KR] b5d6aad89f..., Last passed:
Test 8611
ccextractor --service 1,2[UTF-8],3[EUC-KR],54 --out=txt da904de35d..., Last passed:
Test 8611
ccextractor --autoprogram --out=srt --latin1 f1422b8bfe..., Last passed:
Test 9209
ccextractor --autoprogram --out=srt --latin1 d41b53b504..., Last passed:
Test 8611
ccextractor --stdout --quiet --no-fontcolor 79a51f3500..., Last passed:
Test 8611
ccextractor --stdout --quiet --no-fontcolor 767b546f96..., Last passed:
Test 8611
ccextractor --service 1 c83f765c66..., Last passed:
Test 8611
ccextractor --myth c83f765c66..., Last passed:
Test 8611
ccextractor --in=raw fb79021542..., Last passed:
Test 8611
ccextractor --mp4vidtrack 5df914ce77..., Last passed:
Test 8611
ccextractor --xmltv=3 --out=null 96efd279cf..., Last passed:
Test 8611
ccextractor --datapid 2310 --autoprogram --out=srt --latin1 e639e54550..., Last passed:
Test 8611

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 --out=spupng c83f765c66..., 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.

Shiv0087 and others added 2 commits March 31, 2026 22:45

fix: correct indentation in blockaddition cleanup

bc42bf6

fix: restore free_sub_track loop scope and tab indentation

dd7d640

Copilot AI review requested due to automatic review settings April 22, 2026 09:37

Copilot started reviewing on behalf of Shiv0087 April 22, 2026 09:37 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Comment thread src/lib_ccx/matroska.c Outdated

Comment on lines +1910 to +1913

/* cue_settings_list is the base of the message buffer;

* cue_identifier and comment are pointers into it */

if (sentence->blockaddition->cue_settings_list != NULL)

free(sentence->blockaddition->cue_settings_list);

Shiv0087 requested a review from Copilot April 23, 2026 13:59

Copilot started reviewing on behalf of Shiv0087 April 23, 2026 13:59 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread src/lib_ccx/matroska.c

Comment on lines +1910 to +1914

/* cue_settings_list is the base of the message buffer;

* cue_identifier and comment are pointers into it */

if (sentence->blockaddition->cue_settings_list != NULL)

free(sentence->blockaddition->cue_settings_list);

cfsmp3 requested changes Apr 24, 2026

View reviewed changes

fix: store and free blockaddition message buffer

a73237a

Shiv0087 force-pushed the patch-1 branch from e0dad1d to a73237a Compare April 24, 2026 10:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FIX] Restore free_sub_track scope and keep blockaddition cleanup#2270

[FIX] Restore free_sub_track scope and keep blockaddition cleanup#2270
Shiv0087 wants to merge 3 commits intoCCExtractor:masterfrom
Shiv0087:patch-1

Shiv0087 commented Apr 22, 2026

Uh oh!

Shiv0087 commented Apr 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

cfsmp3 left a comment

Uh oh!

Shiv0087 commented Apr 24, 2026

Uh oh!

ccextractor-bot commented Apr 24, 2026

Uh oh!

ccextractor-bot commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Shiv0087 commented Apr 22, 2026

Summary

What changed

Why

Related

Validation

Uh oh!

Shiv0087 commented Apr 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

cfsmp3 left a comment

Choose a reason for hiding this comment

The issue

Fix

Uh oh!

Shiv0087 commented Apr 24, 2026

Uh oh!

ccextractor-bot commented Apr 24, 2026

Uh oh!

ccextractor-bot commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants