Skip to content

cgo/ffmpeg: free C strings passed to FFmpeg lookups and options - #133

Open
OvOhao wants to merge 1 commit into
deepch:masterfrom
OvOhao:fix-ffmpeg-cstring-leaks
Open

OvOhao wants to merge 1 commit into
deepch:masterfrom
OvOhao:fix-ffmpeg-cstring-leaks

Conversation

@OvOhao

@OvOhao OvOhao commented Oct 21, 2026

Copy link
Copy Markdown

Fixes #130

What changed

Several functions in cgo/ffmpeg passed C.CString(...) directly as a call argument, so the C strings were never freed:

  • HasEncoder, HasDecoder (ffmpeg.go)
  • NewAudioEncoderByName, AudioEncoder.SetOption, AudioEncoder.GetOption (audio.go)
  • the six av_opt_set_int calls in Resampler.Resample (audio.go)

Each string is now bound to a variable and released with defer C.free(...). The six av_opt_set_int calls go through a small avOptSetInt helper that does the same. <stdlib.h> is added to ffmpeg.h for C.free.

Why it is safe

None of the callees keep the pointer after returning:

  • avcodec_find_encoder_by_name / avcodec_find_decoder_by_name only compare the name.
  • avcodec_profile_name_to_int (in ffmpeg.h) only compares the name with strcasecmp.
  • av_dict_get only reads the key; the returned entry points into the dictionary, not into the key.
  • av_dict_set with flags == 0 copies both key and value with av_strdup.
  • av_opt_set_int only uses the name to look up the option.

How it was tested

In a golang:1.22-bullseye container with Debian's FFmpeg 4.3 dev packages (libavformat-dev libavcodec-dev libavutil-dev libavresample-dev libswscale-dev), before and after this change:

  • go build ./cgo/ffmpeg/ and go vet ./cgo/ffmpeg/: both pass.
  • A small program that runs 20000 cycles of HasEncoder("aac"), HasDecoder("aac"), NewAudioEncoderByName("aac"), SetOption("profile", "LC"), SetOption("b", 128000), GetOption("b", &s) and Close(), measuring mallinfo().uordblks around the loop:
before: cycles=20000 malloc in-use delta=4483792 bytes (224.2 B/cycle)
after:  cycles=20000 malloc in-use delta=4144 bytes (0.2 B/cycle)

224 B/cycle matches 7 leaked strings per cycle at glibc's 32-byte minimum chunk. The Resampler path is covered by the build only.

HasEncoder, HasDecoder, NewAudioEncoderByName, AudioEncoder.SetOption,
AudioEncoder.GetOption and the Resampler setup passed C.CString results
directly as arguments, so the strings were never freed. None of the
callees keep the pointer (av_dict_set copies key and value with flags 0),
so free each string after the call.

Fixes deepch#130
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.

Possible memory leak: inline C.CString arguments in cgo/ffmpeg lookups and SetOption/GetOption are never freed

1 participant