Conversation
There was a problem hiding this comment.
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.
| // #cgo LDFLAGS: -ltcmalloc | ||
|
|
There was a problem hiding this comment.
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.
| // #cgo LDFLAGS: -ltcmalloc |
Changes include:
sudo apt-get install libopenjp2-7-devon Ubuntubrew install openjpegon the Mac.jan/enable-libopenjpegon elv-toolchain to build a new ffmpeg with libopenjpeg support