Skip to content

Jan/tc malloc in tests#145

Open
JanFalkin wants to merge 3 commits intomasterfrom
jan/tc-malloc-in-tests
Open

Jan/tc malloc in tests#145
JanFalkin wants to merge 3 commits intomasterfrom
jan/tc-malloc-in-tests

Conversation

@JanFalkin
Copy link
Copy Markdown
Contributor

Changes include:

  1. A bunch of the changes simply remove extra whitespace.
  2. This does require libopenjpeg and gpertools to be installed on the machine that builds ffmpeg or avpipe.
  • sudo apt-get install libopenjp2-7-dev on Ubuntu
  • brew install openjpeg on the Mac.
  1. Includes a debug build for avpipe and test harness for running under tcmalloc heapcheck/profile
  2. ffmpeg and ffprobe will now handle jpeg2000 read and write along with avpipe.
  3. New jpeg2000 version of test file pushed to gcloud. The old one was corrupted and written by our old ffmpeg
  4. Documentation for building and debugging including tcmalloc
  5. Branch jan/enable-libopenjpeg on elv-toolchain to build a new ffmpeg with libopenjpeg support

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds comprehensive memory leak detection tooling using tcmalloc, debug build support, and improves JPEG2000/MXF file handling. The changes enable developers to debug memory issues and build avpipe with debug symbols for better troubleshooting.

  • Added tcmalloc integration scripts for heap checking and leak detection during testing
  • Implemented debug build support for both Go and C components with proper compiler flags
  • Enhanced frame rate detection logic to handle MXF files that lack avg_frame_rate by falling back to r_frame_rate
  • Cleaned up trailing whitespace across multiple C source files

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
run_tests_with_tcmalloc.sh New script for running tests with tcmalloc heap profiling and leak detection
run_tests_leak_check.sh New script for minimal leak detection without extensive profiling
rules.make Added DEBUG flag support for conditional debug compilation flags (-g -O0)
Makefile Added avpipe-debug, debug-libs, and test-tcmalloc targets for debug builds and testing
libavpipe/src/avpipe_xc.c Enhanced frame rate handling with r_frame_rate fallback for MXF files; removed trailing whitespace
avpipe.c Removed trailing whitespace from multiple lines
avpipe_test.go Updated TestMXF_H265MezMaker to remove explicit jpeg2000 decoder and add BitDepth parameter
avpipe.go Added tcmalloc CGO linking directive
TCMALLOC_README.md Comprehensive documentation for tcmalloc usage, environment variables, and troubleshooting
DEBUG_BUILD_README.md Complete guide for building with debug symbols, using GDB, and analyzing core dumps

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

Comment thread avpipe.go
Comment on lines +32 to 33
// #cgo LDFLAGS: -ltcmalloc

Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a hard link to tcmalloc via CGO LDFLAGS may cause build failures on systems where tcmalloc is not installed or available. According to the PR description and documentation (TCMALLOC_README.md), the preferred approach is to use LD_PRELOAD for tcmalloc injection, which is more flexible and doesn't require linking at build time. Consider removing this line and relying solely on the LD_PRELOAD approach documented in the scripts.

Suggested change
// #cgo LDFLAGS: -ltcmalloc

Copilot uses AI. Check for mistakes.
Comment thread avpipe_test.go Outdated
Comment thread run_tests_leak_check.sh
Comment thread run_tests_leak_check.sh Outdated
Comment thread run_tests_leak_check.sh Outdated
Comment thread run_tests_with_tcmalloc.sh Outdated
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