From 85a63d475e641a98b2a575854bd9d92c779b2822 Mon Sep 17 00:00:00 2001 From: Nicolas Hug Date: Sat, 12 Jul 2025 21:32:49 +0100 Subject: [PATCH 1/4] Fix?? --- src/torchcodec/_core/Encoder.cpp | 16 ++++++++++++++-- src/torchcodec/_core/Encoder.h | 1 + 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/torchcodec/_core/Encoder.cpp b/src/torchcodec/_core/Encoder.cpp index 5a6a0d7e..2c7d275c 100644 --- a/src/torchcodec/_core/Encoder.cpp +++ b/src/torchcodec/_core/Encoder.cpp @@ -99,8 +99,18 @@ AVSampleFormat findBestOutputSampleFormat(const AVCodec& avCodec) { } // namespace AudioEncoder::~AudioEncoder() { - if (avFormatContext_ && avFormatContext_->pb && !avioContextHolder_) { - avio_close(avFormatContext_->pb); + close_avio(); +} + +void AudioEncoder::close_avio() { + if (avFormatContext_ && avFormatContext_->pb) { + avio_flush(avFormatContext_->pb); + + if (!avioContextHolder_) { + avio_close(avFormatContext_->pb); + // avoids closing again in destructor, which would segfault. + avFormatContext_->pb = nullptr; + } } } @@ -308,6 +318,8 @@ void AudioEncoder::encode() { status == AVSUCCESS, "Error in: av_write_trailer", getFFMPEGErrorStringFromErrorCode(status)); + + close_avio(); } UniqueAVFrame AudioEncoder::maybeConvertAVFrame(const UniqueAVFrame& avFrame) { diff --git a/src/torchcodec/_core/Encoder.h b/src/torchcodec/_core/Encoder.h index 04ed6a13..723849f0 100644 --- a/src/torchcodec/_core/Encoder.h +++ b/src/torchcodec/_core/Encoder.h @@ -33,6 +33,7 @@ class AudioEncoder { void encodeFrame(AutoAVPacket& autoAVPacket, const UniqueAVFrame& avFrame); void maybeFlushSwrBuffers(AutoAVPacket& autoAVPacket); void flushBuffers(); + void close_avio(); UniqueEncodingAVFormatContext avFormatContext_; UniqueAVCodecContext avCodecContext_; From 801dda6e2304a59ed95096e8ce876f34664a8d2e Mon Sep 17 00:00:00 2001 From: Nicolas Hug Date: Sat, 12 Jul 2025 21:37:59 +0100 Subject: [PATCH 2/4] Debug buffer error --- .github/workflows/linux_cuda_wheel.yaml | 2 +- .github/workflows/linux_wheel.yaml | 2 +- .github/workflows/macos_wheel.yaml | 2 +- test/test_encoders.py | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/linux_cuda_wheel.yaml b/.github/workflows/linux_cuda_wheel.yaml index bd57cac5..7329e460 100644 --- a/.github/workflows/linux_cuda_wheel.yaml +++ b/.github/workflows/linux_cuda_wheel.yaml @@ -137,7 +137,7 @@ jobs: ls - name: Run Python tests run: | - ${CONDA_RUN} FAIL_WITHOUT_CUDA=1 pytest test -v --tb=short + ${CONDA_RUN} FAIL_WITHOUT_CUDA=1 pytest test -v --tb=short -k test_num_channels - name: Run Python benchmark run: | ${CONDA_RUN} time python benchmarks/decoders/gpu_benchmark.py --devices=cuda:0,cpu --resize_devices=none diff --git a/.github/workflows/linux_wheel.yaml b/.github/workflows/linux_wheel.yaml index 1855e904..eaaa4235 100644 --- a/.github/workflows/linux_wheel.yaml +++ b/.github/workflows/linux_wheel.yaml @@ -123,4 +123,4 @@ jobs: ls - name: Run Python tests run: | - pytest test -vvv + pytest test -vvv -k test_num_channels diff --git a/.github/workflows/macos_wheel.yaml b/.github/workflows/macos_wheel.yaml index ee436b7a..2c39e9dc 100644 --- a/.github/workflows/macos_wheel.yaml +++ b/.github/workflows/macos_wheel.yaml @@ -122,4 +122,4 @@ jobs: - name: Run Python tests run: | - pytest test -vvv + pytest test -vvv -k test_num_channels diff --git a/test/test_encoders.py b/test/test_encoders.py index d432263a..d42d0369 100644 --- a/test/test_encoders.py +++ b/test/test_encoders.py @@ -367,8 +367,9 @@ def test_contiguity(self): @pytest.mark.parametrize("num_channels_input", (1, 2)) @pytest.mark.parametrize("num_channels_output", (1, 2, None)) @pytest.mark.parametrize("method", ("to_file", "to_tensor")) + @pytest.mark.parametrize("i", range(1000)) def test_num_channels( - self, num_channels_input, num_channels_output, method, tmp_path + self, num_channels_input, num_channels_output, method, tmp_path, i ): # We just check that the num_channels parameter is respected. # Correctness is checked in other tests (like test_against_cli()) From f14bb1dbc509946d2d0ac2f3a5def3654a028f21 Mon Sep 17 00:00:00 2001 From: Nicolas Hug Date: Sat, 12 Jul 2025 21:56:01 +0100 Subject: [PATCH 3/4] Lint --- src/torchcodec/_core/Encoder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/torchcodec/_core/Encoder.cpp b/src/torchcodec/_core/Encoder.cpp index 2c7d275c..ce33613c 100644 --- a/src/torchcodec/_core/Encoder.cpp +++ b/src/torchcodec/_core/Encoder.cpp @@ -109,7 +109,7 @@ void AudioEncoder::close_avio() { if (!avioContextHolder_) { avio_close(avFormatContext_->pb); // avoids closing again in destructor, which would segfault. - avFormatContext_->pb = nullptr; + avFormatContext_->pb = nullptr; } } } From 492d6e9a573a963aa87aede0cc8ba552566bdc9f Mon Sep 17 00:00:00 2001 From: Nicolas Hug Date: Sat, 12 Jul 2025 21:56:55 +0100 Subject: [PATCH 4/4] test 10k times --- test/test_encoders.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_encoders.py b/test/test_encoders.py index d42d0369..73410831 100644 --- a/test/test_encoders.py +++ b/test/test_encoders.py @@ -367,7 +367,7 @@ def test_contiguity(self): @pytest.mark.parametrize("num_channels_input", (1, 2)) @pytest.mark.parametrize("num_channels_output", (1, 2, None)) @pytest.mark.parametrize("method", ("to_file", "to_tensor")) - @pytest.mark.parametrize("i", range(1000)) + @pytest.mark.parametrize("i", range(10_000)) def test_num_channels( self, num_channels_input, num_channels_output, method, tmp_path, i ):