CCExtractor version: master (907be05)
Necessary information
- Is this a regression (i.e. did it work before)? NO — present since
--webvtt-create-css was added.
- What platform did you use? All (platform-independent; this is a logic bug, not a portability one)
- What were the used arguments?
--out=webvtt --webvtt-create-css
Video links
Not needed — any input that produces WebVTT output reproduces this. The trigger is the output directory, not the input file. Steps are below.
Additional information
write_webvtt_header() in src/lib_ccx/ccx_encoders_webvtt.c has two defects in its --webvtt-create-css path.
1. Header re-emitted before every cue when the CSS file can't be created
https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_encoders_webvtt.c#L255-L261
FILE *f = fopen(css_file_name, "wb");
if (f == NULL)
{
mprint("Warning: Error creating the file %s\n", css_file_name);
free(css_file_name);
return; // <-- skips wrote_webvtt_header = 1
}
The function guards its own re-entry with if (context->wrote_webvtt_header) return; at the top, and sets that flag on the last line:
context->wrote_webvtt_header = 1; // Do it even if couldn't write the header, because it won't be possible anyway
That trailing comment states the intent explicitly: the flag should be set even when the header could not be fully written. The return above bypasses it.
write_webvtt_header() is called from write_stringz_as_webvtt(), write_cc_bitmap_as_webvtt() and write_cc_buffer_as_webvtt() — i.e. once per cue. So when the CSS file cannot be created, the flag never gets set and the whole header block (either the X-TIMESTAMP-MAP line or the blank line, depending on --timestamp-map) is written again before every subsequent cue, in the middle of the file. The resulting .vtt is not valid WebVTT.
Repro:
mkdir ro && cd ro
ccextractor ../any_input.ts --out=webvtt --webvtt-create-css -o out.vtt
# make the CSS creation fail, e.g. by making the CWD read-only first:
# chmod a-w . (Linux/macOS)
# then inspect out.vtt - the header block repeats before each cue
(fopen for the .css uses the current working directory, not the output path, so a read-only CWD is enough — which is itself a little surprising.)
2. basefilename is leaked on every path
https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_encoders_webvtt.c#L247
char *basefilename = get_basename(context->first_input_file);
get_basename() returns malloced memory (utility.c), but basefilename is never freed — not on the success path, not on the fopen-failure path. Every other caller in the tree frees it (ccx_encoders_common.c:607, :636, :1369, :1437).
Normally that is a single small leak per output file. Combined with defect 1 it becomes a leak per cue, since the function then re-runs for the whole recording.
3. Minor: unguarded get_basename() return
get_basename() returns NULL if its argument is NULL or on allocation failure, and strlen(basefilename) on the next line would then crash. ccx_encoders_common.c:636-641 guards exactly this case:
char *basefilename = get_basename(ctx->first_input_file);
...
if (basefilename == NULL)
{
basefilename = get_basename("untitled");
}
To be straight about this one: I could not establish that first_input_file is ever actually NULL on a live run — the Rust parser leaves it as an empty string rather than NULL (parser.rs:1618-1620), so in practice get_basename() returns "". I'm flagging it as hardening consistent with the existing guard, not as a demonstrated crash. Happy to drop it from the patch if you'd rather keep the diff to the two real defects.
Verification
I confirmed defects 1 and 2 by reading the code and compiling; both are static facts about the control flow (a return that skips an assignment, and an allocation with no matching free) rather than behavioural guesses, so they're checkable in about twenty lines. I did not execute the fopen-failure path, since I don't have a full local build — so I'd appreciate a sanity check on the repro above rather than taking my word for the runtime effect.
PR follows.
CCExtractor version: master (907be05)
Necessary information
--webvtt-create-csswas added.--out=webvtt --webvtt-create-cssVideo links
Not needed — any input that produces WebVTT output reproduces this. The trigger is the output directory, not the input file. Steps are below.
Additional information
write_webvtt_header()insrc/lib_ccx/ccx_encoders_webvtt.chas two defects in its--webvtt-create-csspath.1. Header re-emitted before every cue when the CSS file can't be created
https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_encoders_webvtt.c#L255-L261
The function guards its own re-entry with
if (context->wrote_webvtt_header) return;at the top, and sets that flag on the last line:That trailing comment states the intent explicitly: the flag should be set even when the header could not be fully written. The
returnabove bypasses it.write_webvtt_header()is called fromwrite_stringz_as_webvtt(),write_cc_bitmap_as_webvtt()andwrite_cc_buffer_as_webvtt()— i.e. once per cue. So when the CSS file cannot be created, the flag never gets set and the whole header block (either theX-TIMESTAMP-MAPline or the blank line, depending on--timestamp-map) is written again before every subsequent cue, in the middle of the file. The resulting.vttis not valid WebVTT.Repro:
(
fopenfor the.cssuses the current working directory, not the output path, so a read-only CWD is enough — which is itself a little surprising.)2.
basefilenameis leaked on every pathhttps://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_encoders_webvtt.c#L247
get_basename()returnsmalloced memory (utility.c), butbasefilenameis never freed — not on the success path, not on thefopen-failure path. Every other caller in the tree frees it (ccx_encoders_common.c:607,:636,:1369,:1437).Normally that is a single small leak per output file. Combined with defect 1 it becomes a leak per cue, since the function then re-runs for the whole recording.
3. Minor: unguarded
get_basename()returnget_basename()returnsNULLif its argument isNULLor on allocation failure, andstrlen(basefilename)on the next line would then crash.ccx_encoders_common.c:636-641guards exactly this case:To be straight about this one: I could not establish that
first_input_fileis ever actuallyNULLon a live run — the Rust parser leaves it as an empty string rather thanNULL(parser.rs:1618-1620), so in practiceget_basename()returns"". I'm flagging it as hardening consistent with the existing guard, not as a demonstrated crash. Happy to drop it from the patch if you'd rather keep the diff to the two real defects.Verification
I confirmed defects 1 and 2 by reading the code and compiling; both are static facts about the control flow (a
returnthat skips an assignment, and an allocation with no matchingfree) rather than behavioural guesses, so they're checkable in about twenty lines. I did not execute thefopen-failure path, since I don't have a full local build — so I'd appreciate a sanity check on the repro above rather than taking my word for the runtime effect.PR follows.