From 64d1a756f72547267dff0deb1645f35dcc824d61 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Wed, 15 Jan 2025 20:53:40 -0800 Subject: [PATCH 01/17] Turn on all C++ warnings and make them errors --- src/torchcodec/decoders/_core/CMakeLists.txt | 1 + .../decoders/_core/CPUOnlyDevice.cpp | 14 +++++----- src/torchcodec/decoders/_core/CudaDevice.cpp | 6 ++--- .../decoders/_core/FFMPEGCommon.cpp | 2 +- .../decoders/_core/VideoDecoder.cpp | 26 ++++++++++--------- src/torchcodec/decoders/_core/VideoDecoder.h | 2 +- .../decoders/_core/VideoDecoderOps.cpp | 3 ++- 7 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/torchcodec/decoders/_core/CMakeLists.txt b/src/torchcodec/decoders/_core/CMakeLists.txt index 22bcd654a..a0782c523 100644 --- a/src/torchcodec/decoders/_core/CMakeLists.txt +++ b/src/torchcodec/decoders/_core/CMakeLists.txt @@ -2,6 +2,7 @@ cmake_minimum_required(VERSION 3.18) project(TorchCodec) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) +add_compile_options(-Wall -Wextra -pedantic -Werror -Wparentheses) find_package(Torch REQUIRED) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${TORCH_CXX_FLAGS}") diff --git a/src/torchcodec/decoders/_core/CPUOnlyDevice.cpp b/src/torchcodec/decoders/_core/CPUOnlyDevice.cpp index 7d058130f..213d507ef 100644 --- a/src/torchcodec/decoders/_core/CPUOnlyDevice.cpp +++ b/src/torchcodec/decoders/_core/CPUOnlyDevice.cpp @@ -16,28 +16,28 @@ namespace facebook::torchcodec { void convertAVFrameToDecodedOutputOnCuda( const torch::Device& device, - const VideoDecoder::VideoStreamDecoderOptions& options, - VideoDecoder::RawDecodedOutput& rawOutput, - VideoDecoder::DecodedOutput& output, - std::optional preAllocatedOutputTensor) { + [[maybe_unused]] const VideoDecoder::VideoStreamDecoderOptions& options, + [[maybe_unused]] VideoDecoder::RawDecodedOutput& rawOutput, + [[maybe_unused]] VideoDecoder::DecodedOutput& output, + [[maybe_unused]] std::optional preAllocatedOutputTensor) { throwUnsupportedDeviceError(device); } void initializeContextOnCuda( const torch::Device& device, - AVCodecContext* codecContext) { + [[maybe_unused]] AVCodecContext* codecContext) { throwUnsupportedDeviceError(device); } void releaseContextOnCuda( const torch::Device& device, - AVCodecContext* codecContext) { + [[maybe_unused]] AVCodecContext* codecContext) { throwUnsupportedDeviceError(device); } std::optional findCudaCodec( const torch::Device& device, - const AVCodecID& codecId) { + [[maybe_unused]] const AVCodecID& codecId) { throwUnsupportedDeviceError(device); } diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index 69fef471f..d0924a02a 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -265,10 +265,10 @@ std::optional findCudaCodec( const AVCodecID& codecId) { throwErrorIfNonCudaDevice(device); - void* i = NULL; + void* i = nullptr; - AVCodecPtr c; - while (c = av_codec_iterate(&i)) { + const AVCodecPtr c; + while ((c = av_codec_iterate(&i))) { const AVCodecHWConfig* config; if (c->id != codecId || !av_codec_is_decoder(c)) { diff --git a/src/torchcodec/decoders/_core/FFMPEGCommon.cpp b/src/torchcodec/decoders/_core/FFMPEGCommon.cpp index ab50414fc..64526fb7d 100644 --- a/src/torchcodec/decoders/_core/FFMPEGCommon.cpp +++ b/src/torchcodec/decoders/_core/FFMPEGCommon.cpp @@ -21,7 +21,7 @@ int64_t getDuration(const UniqueAVFrame& frame) { } int64_t getDuration(const AVFrame* frame) { -#if LIBAVUTIL_VERSION_MAJOR < 59 +#if LIBAVUTIL_VERSION_MAJOR < 58 return frame->pkt_duration; #else return frame->duration; diff --git a/src/torchcodec/decoders/_core/VideoDecoder.cpp b/src/torchcodec/decoders/_core/VideoDecoder.cpp index 737cf4789..520977a90 100644 --- a/src/torchcodec/decoders/_core/VideoDecoder.cpp +++ b/src/torchcodec/decoders/_core/VideoDecoder.cpp @@ -206,7 +206,8 @@ VideoDecoder::BatchDecodedOutput::BatchDecodedOutput( bool VideoDecoder::DecodedFrameContext::operator==( const VideoDecoder::DecodedFrameContext& other) { - return decodedWidth == other.decodedWidth && decodedHeight == decodedHeight && + return decodedWidth == other.decodedWidth && + decodedHeight == other.decodedHeight && decodedFormat == other.decodedFormat && expectedWidth == other.expectedWidth && expectedHeight == other.expectedHeight; @@ -249,12 +250,12 @@ void VideoDecoder::initializeDecoder() { getFFMPEGErrorStringFromErrorCode(ffmpegStatus)); } - for (int i = 0; i < formatContext_->nb_streams; i++) { + for (unsigned int i = 0; i < formatContext_->nb_streams; i++) { AVStream* stream = formatContext_->streams[i]; StreamMetadata meta; TORCH_CHECK( - i == stream->index, + static_cast(i) == stream->index, "Our stream index, " + std::to_string(i) + ", does not match AVStream's index, " + std::to_string(stream->index) + "."); @@ -577,7 +578,7 @@ void VideoDecoder::scanFileAndUpdateMetadataAndIndex() { } streams_[streamIndex].allFrames.push_back(frameInfo); } - for (int i = 0; i < containerMetadata_.streams.size(); ++i) { + for (size_t i = 0; i < containerMetadata_.streams.size(); ++i) { auto& streamMetadata = containerMetadata_.streams[i]; auto stream = formatContext_->streams[i]; if (streamMetadata.minPtsFromScan.has_value()) { @@ -610,7 +611,7 @@ void VideoDecoder::scanFileAndUpdateMetadataAndIndex() { return frameInfo1.pts < frameInfo2.pts; }); - for (int i = 0; i < stream.allFrames.size(); ++i) { + for (size_t i = 0; i < stream.allFrames.size(); ++i) { if (i + 1 < stream.allFrames.size()) { stream.allFrames[i].nextPts = stream.allFrames[i + 1].pts; } @@ -1050,8 +1051,8 @@ VideoDecoder::DecodedOutput VideoDecoder::getFramePlayedAtTimestampNoDemux( return output; } -void VideoDecoder::validateUserProvidedStreamIndex(uint64_t streamIndex) { - size_t streamsSize = containerMetadata_.streams.size(); +void VideoDecoder::validateUserProvidedStreamIndex(int streamIndex) { + int streamsSize = static_cast(containerMetadata_.streams.size()); TORCH_CHECK( streamIndex >= 0 && streamIndex < streamsSize, "Invalid stream index=" + std::to_string(streamIndex) + @@ -1074,7 +1075,7 @@ void VideoDecoder::validateFrameIndex( const StreamInfo& stream, int64_t frameIndex) { TORCH_CHECK( - frameIndex >= 0 && frameIndex < stream.allFrames.size(), + frameIndex >= 0 && frameIndex < static_cast(stream.allFrames.size()), "Invalid frame index=" + std::to_string(frameIndex) + " for streamIndex=" + std::to_string(stream.streamIndex) + " numFrames=" + std::to_string(stream.allFrames.size())); @@ -1134,10 +1135,11 @@ VideoDecoder::BatchDecodedOutput VideoDecoder::getFramesAtIndices( BatchDecodedOutput output(frameIndices.size(), options, streamMetadata); auto previousIndexInVideo = -1; - for (auto f = 0; f < frameIndices.size(); ++f) { + for (size_t f = 0; f < frameIndices.size(); ++f) { auto indexInOutput = indicesAreSorted ? f : argsort[f]; auto indexInVideo = frameIndices[indexInOutput]; - if (indexInVideo < 0 || indexInVideo >= stream.allFrames.size()) { + if (indexInVideo < 0 || + indexInVideo >= static_cast(stream.allFrames.size())) { throw std::runtime_error( "Invalid frame index=" + std::to_string(indexInVideo)); } @@ -1180,7 +1182,7 @@ VideoDecoder::BatchDecodedOutput VideoDecoder::getFramesPlayedByTimestamps( double maxSeconds = streamMetadata.maxPtsSecondsFromScan.value(); std::vector frameIndices(timestamps.size()); - for (auto i = 0; i < timestamps.size(); ++i) { + for (size_t i = 0; i < timestamps.size(); ++i) { auto framePts = timestamps[i]; TORCH_CHECK( framePts >= minSeconds && framePts < maxSeconds, @@ -1215,7 +1217,7 @@ VideoDecoder::BatchDecodedOutput VideoDecoder::getFramesInRange( TORCH_CHECK( start >= 0, "Range start, " + std::to_string(start) + " is less than 0."); TORCH_CHECK( - stop <= stream.allFrames.size(), + stop <= static_cast(stream.allFrames.size()), "Range stop, " + std::to_string(stop) + ", is more than the number of frames, " + std::to_string(stream.allFrames.size())); diff --git a/src/torchcodec/decoders/_core/VideoDecoder.h b/src/torchcodec/decoders/_core/VideoDecoder.h index 56d83fbfa..7d828bf2b 100644 --- a/src/torchcodec/decoders/_core/VideoDecoder.h +++ b/src/torchcodec/decoders/_core/VideoDecoder.h @@ -366,7 +366,7 @@ class VideoDecoder { // for more details about the heuristics. int getBestStreamIndex(AVMediaType mediaType); void initializeDecoder(); - void validateUserProvidedStreamIndex(uint64_t streamIndex); + void validateUserProvidedStreamIndex(int streamIndex); void validateScannedAllStreams(const std::string& msg); void validateFrameIndex(const StreamInfo& stream, int64_t frameIndex); // Creates and initializes a filter graph for a stream. The filter graph can diff --git a/src/torchcodec/decoders/_core/VideoDecoderOps.cpp b/src/torchcodec/decoders/_core/VideoDecoderOps.cpp index 7f6bd3b36..5d9414477 100644 --- a/src/torchcodec/decoders/_core/VideoDecoderOps.cpp +++ b/src/torchcodec/decoders/_core/VideoDecoderOps.cpp @@ -396,7 +396,8 @@ std::string get_stream_json_metadata( int64_t stream_index) { auto videoDecoder = unwrapTensorToGetDecoder(decoder); auto streams = videoDecoder->getContainerMetadata().streams; - if (stream_index < 0 || stream_index >= streams.size()) { + if (stream_index < 0 || + stream_index >= static_cast(streams.size())) { throw std::out_of_range( "stream_index out of bounds: " + std::to_string(stream_index)); } From c228588499b5752aa632aa50bcfe1d447bba144f Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Wed, 15 Jan 2025 20:57:39 -0800 Subject: [PATCH 02/17] Wparenthesis is implied by Wall --- src/torchcodec/decoders/_core/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/torchcodec/decoders/_core/CMakeLists.txt b/src/torchcodec/decoders/_core/CMakeLists.txt index a0782c523..285201fd3 100644 --- a/src/torchcodec/decoders/_core/CMakeLists.txt +++ b/src/torchcodec/decoders/_core/CMakeLists.txt @@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.18) project(TorchCodec) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) -add_compile_options(-Wall -Wextra -pedantic -Werror -Wparentheses) +add_compile_options(-Wall -Wextra -pedantic -Werror) find_package(Torch REQUIRED) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${TORCH_CXX_FLAGS}") From 07ef5793d6cf342cd1c120de38df43bb3e47ffbc Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Wed, 15 Jan 2025 21:10:33 -0800 Subject: [PATCH 03/17] Cast values passed to FFMIN --- src/torchcodec/decoders/_core/FFMPEGCommon.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/torchcodec/decoders/_core/FFMPEGCommon.cpp b/src/torchcodec/decoders/_core/FFMPEGCommon.cpp index 64526fb7d..113eae35f 100644 --- a/src/torchcodec/decoders/_core/FFMPEGCommon.cpp +++ b/src/torchcodec/decoders/_core/FFMPEGCommon.cpp @@ -75,7 +75,8 @@ int AVIOBytesContext::read(void* opaque, uint8_t* buf, int buf_size) { bufferData->current, ", size=", bufferData->size); - buf_size = FFMIN(buf_size, bufferData->size - bufferData->current); + buf_size = + FFMIN(buf_size, static_cast(bufferData->size - bufferData->current)); TORCH_CHECK( buf_size >= 0, "Tried to read negative bytes: buf_size=", From 0b8e9e0e0bf2cf3c2f3a6f5f7738000b0904f237 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 06:16:15 -0800 Subject: [PATCH 04/17] Use CXX_FLAGS; fix Cuda code --- src/torchcodec/decoders/_core/CMakeLists.txt | 3 +-- src/torchcodec/decoders/_core/CudaDevice.cpp | 13 ++++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/torchcodec/decoders/_core/CMakeLists.txt b/src/torchcodec/decoders/_core/CMakeLists.txt index 285201fd3..717e9adf1 100644 --- a/src/torchcodec/decoders/_core/CMakeLists.txt +++ b/src/torchcodec/decoders/_core/CMakeLists.txt @@ -2,10 +2,9 @@ cmake_minimum_required(VERSION 3.18) project(TorchCodec) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) -add_compile_options(-Wall -Wextra -pedantic -Werror) find_package(Torch REQUIRED) -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${TORCH_CXX_FLAGS}") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${TORCH_CXX_FLAGS} -Wall -Wextra -pedantic -Werror") find_package(Python3 ${PYTHON_VERSION} EXACT COMPONENTS Development) function(make_torchcodec_library library_name ffmpeg_target) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index d0924a02a..769f5f50d 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -267,17 +267,16 @@ std::optional findCudaCodec( void* i = nullptr; - const AVCodecPtr c; - while ((c = av_codec_iterate(&i))) { - const AVCodecHWConfig* config; - - if (c->id != codecId || !av_codec_is_decoder(c)) { + while ((const AVCodecPtr codec = av_codec_iterate(&i))) { + if (codec->id != codecId || !av_codec_is_decoder(codec)) { continue; } - for (int j = 0; config = avcodec_get_hw_config(c, j); j++) { + for (int j = 0; + (const AVCodecHWConfig* config = avcodec_get_hw_config(codec, j)); + j++) { if (config->device_type == AV_HWDEVICE_TYPE_CUDA) { - return c; + return codec; } } } From 22c6957d28d6d44f4715ea1bcba60446b75a7378 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 06:17:35 -0800 Subject: [PATCH 05/17] Spacing --- src/torchcodec/decoders/_core/CudaDevice.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index 769f5f50d..6bb1e0f42 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -266,7 +266,6 @@ std::optional findCudaCodec( throwErrorIfNonCudaDevice(device); void* i = nullptr; - while ((const AVCodecPtr codec = av_codec_iterate(&i))) { if (codec->id != codecId || !av_codec_is_decoder(codec)) { continue; From 5a3aaba49a60bd8b8a818d2e747a63ba0b7f606c Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 06:17:35 -0800 Subject: [PATCH 06/17] Spacing --- src/torchcodec/decoders/_core/CudaDevice.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index 769f5f50d..2796ffefd 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -260,14 +260,13 @@ void convertAVFrameToDecodedOutputOnCuda( // we have to do this because of an FFmpeg bug where hardware decoding is not // appropriately set, so we just go off and find the matching codec for the CUDA // device -std::optional findCudaCodec( +std::optional findCudaCodec( const torch::Device& device, const AVCodecID& codecId) { throwErrorIfNonCudaDevice(device); void* i = nullptr; - - while ((const AVCodecPtr codec = av_codec_iterate(&i))) { + while ((const AVCodec* codec = av_codec_iterate(&i))) { if (codec->id != codecId || !av_codec_is_decoder(codec)) { continue; } From 36f6321e4d6a9115df4345ad8579a50c2c6088da Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 06:47:56 -0800 Subject: [PATCH 07/17] Parentheses --- src/torchcodec/decoders/_core/CudaDevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index 2796ffefd..e09480593 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -266,7 +266,7 @@ std::optional findCudaCodec( throwErrorIfNonCudaDevice(device); void* i = nullptr; - while ((const AVCodec* codec = av_codec_iterate(&i))) { + while (const AVCodec* codec = av_codec_iterate(&i)) { if (codec->id != codecId || !av_codec_is_decoder(codec)) { continue; } From 4883b03e55356fced7aa39af947dcd6cdc753ef9 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 07:05:24 -0800 Subject: [PATCH 08/17] Remove const? --- src/torchcodec/decoders/_core/CudaDevice.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index e09480593..55f4ab47c 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -266,13 +266,13 @@ std::optional findCudaCodec( throwErrorIfNonCudaDevice(device); void* i = nullptr; - while (const AVCodec* codec = av_codec_iterate(&i)) { + while ((AVCodec* codec = av_codec_iterate(&i)) != nullptr) { if (codec->id != codecId || !av_codec_is_decoder(codec)) { continue; } for (int j = 0; - (const AVCodecHWConfig* config = avcodec_get_hw_config(codec, j)); + (AVCodecHWConfig* config = avcodec_get_hw_config(codec, j)) != nullptr; j++) { if (config->device_type == AV_HWDEVICE_TYPE_CUDA) { return codec; From a4df539e0092657465e2f519640bf71b6cbb6b53 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 07:40:32 -0800 Subject: [PATCH 09/17] Move declarations out of while loop --- src/torchcodec/decoders/_core/CudaDevice.cpp | 10 +++++----- src/torchcodec/decoders/_core/DeviceInterface.h | 4 ---- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index 55f4ab47c..6ab809615 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -9,7 +9,6 @@ #include "src/torchcodec/decoders/_core/VideoDecoder.h" extern "C" { -#include #include #include } @@ -266,14 +265,15 @@ std::optional findCudaCodec( throwErrorIfNonCudaDevice(device); void* i = nullptr; - while ((AVCodec* codec = av_codec_iterate(&i)) != nullptr) { + const AVCodec** codec = nullptr; + while ((*codec = av_codec_iterate(&i)) != nullptr) { if (codec->id != codecId || !av_codec_is_decoder(codec)) { continue; } - for (int j = 0; - (AVCodecHWConfig* config = avcodec_get_hw_config(codec, j)) != nullptr; - j++) { + const AVCodecHWConfig** config = nullptr; + for (int j = 0; (*config = avcodec_get_hw_config(codec, j)) != nullptr; + ++j) { if (config->device_type == AV_HWDEVICE_TYPE_CUDA) { return codec; } diff --git a/src/torchcodec/decoders/_core/DeviceInterface.h b/src/torchcodec/decoders/_core/DeviceInterface.h index 289308cbc..5ae201d21 100644 --- a/src/torchcodec/decoders/_core/DeviceInterface.h +++ b/src/torchcodec/decoders/_core/DeviceInterface.h @@ -13,10 +13,6 @@ #include "FFMPEGCommon.h" #include "src/torchcodec/decoders/_core/VideoDecoder.h" -extern "C" { -#include -} - namespace facebook::torchcodec { // Note that all these device functions should only be called if the device is From fef1ea01ad4df55d036ef098779180e7cb8f8b45 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 07:53:14 -0800 Subject: [PATCH 10/17] Pointers are pointes --- src/torchcodec/decoders/_core/CudaDevice.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index 6ab809615..4002bf393 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -267,15 +267,15 @@ std::optional findCudaCodec( void* i = nullptr; const AVCodec** codec = nullptr; while ((*codec = av_codec_iterate(&i)) != nullptr) { - if (codec->id != codecId || !av_codec_is_decoder(codec)) { + if (*codec->id != codecId || !av_codec_is_decoder(*codec)) { continue; } const AVCodecHWConfig** config = nullptr; - for (int j = 0; (*config = avcodec_get_hw_config(codec, j)) != nullptr; + for (int j = 0; (*config = avcodec_get_hw_config(*codec, j)) != nullptr; ++j) { - if (config->device_type == AV_HWDEVICE_TYPE_CUDA) { - return codec; + if (*config->device_type == AV_HWDEVICE_TYPE_CUDA) { + return *codec; } } } From 75b39d6e4fea48733a1fd6a49111a94fb56f624e Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 08:01:36 -0800 Subject: [PATCH 11/17] -> binds tighter than * --- src/torchcodec/decoders/_core/CudaDevice.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index 4002bf393..4ceeae712 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -267,14 +267,14 @@ std::optional findCudaCodec( void* i = nullptr; const AVCodec** codec = nullptr; while ((*codec = av_codec_iterate(&i)) != nullptr) { - if (*codec->id != codecId || !av_codec_is_decoder(*codec)) { + if ((*codec)->id != codecId || !av_codec_is_decoder(*codec)) { continue; } const AVCodecHWConfig** config = nullptr; for (int j = 0; (*config = avcodec_get_hw_config(*codec, j)) != nullptr; ++j) { - if (*config->device_type == AV_HWDEVICE_TYPE_CUDA) { + if ((*config)->device_type == AV_HWDEVICE_TYPE_CUDA) { return *codec; } } From c85f9e66b507c624e2658aa4c58b32bca8ddab68 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 08:13:17 -0800 Subject: [PATCH 12/17] Mark param as maybe unused --- src/torchcodec/decoders/_core/CudaDevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index 4ceeae712..acbb5f384 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -109,7 +109,7 @@ AVBufferRef* getFFMPEGContextFromExistingCudaContext( #else AVBufferRef* getFFMPEGContextFromNewCudaContext( - const torch::Device& device, + [[maybe_unused]] const torch::Device& device, torch::DeviceIndex nonNegativeDeviceIndex, enum AVHWDeviceType type) { AVBufferRef* hw_device_ctx = nullptr; From 0667dd7dff95044c34a810fb79699878fd1aca4c Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 08:31:52 -0800 Subject: [PATCH 13/17] Upgrade docs workflow to upload-artifact v4. --- .github/workflows/docs.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index 6aa6e3f62..59ddf382d 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -112,7 +112,7 @@ jobs: run: | cd docs ${CONDA_RUN} make html - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: name: Built-Docs path: docs/build/html/ From b846335d102360c41d8f3174e538a0473fb60c65 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 08:59:25 -0800 Subject: [PATCH 14/17] Use a unique pointer; actually allocate memory for double pointer --- src/torchcodec/decoders/_core/CudaDevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index acbb5f384..aca33b427 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -265,7 +265,7 @@ std::optional findCudaCodec( throwErrorIfNonCudaDevice(device); void* i = nullptr; - const AVCodec** codec = nullptr; + std::unique_ptr codec(new AVCodec*); while ((*codec = av_codec_iterate(&i)) != nullptr) { if ((*codec)->id != codecId || !av_codec_is_decoder(*codec)) { continue; From 3097e7403a2c65d546f5b9b35661c8b7024e7b09 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 09:00:23 -0800 Subject: [PATCH 15/17] Const correctness --- src/torchcodec/decoders/_core/CudaDevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index aca33b427..487145c21 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -265,7 +265,7 @@ std::optional findCudaCodec( throwErrorIfNonCudaDevice(device); void* i = nullptr; - std::unique_ptr codec(new AVCodec*); + std::unique_ptr codec(new const AVCodec*); while ((*codec = av_codec_iterate(&i)) != nullptr) { if ((*codec)->id != codecId || !av_codec_is_decoder(*codec)) { continue; From bf68d423d240a7e29a417992a5f75d666fe27d94 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Thu, 16 Jan 2025 10:09:31 -0800 Subject: [PATCH 16/17] Simple const pointer --- src/torchcodec/decoders/_core/CudaDevice.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/torchcodec/decoders/_core/CudaDevice.cpp b/src/torchcodec/decoders/_core/CudaDevice.cpp index 487145c21..a786be907 100644 --- a/src/torchcodec/decoders/_core/CudaDevice.cpp +++ b/src/torchcodec/decoders/_core/CudaDevice.cpp @@ -265,17 +265,17 @@ std::optional findCudaCodec( throwErrorIfNonCudaDevice(device); void* i = nullptr; - std::unique_ptr codec(new const AVCodec*); - while ((*codec = av_codec_iterate(&i)) != nullptr) { - if ((*codec)->id != codecId || !av_codec_is_decoder(*codec)) { + const AVCodec* codec = nullptr; + while ((codec = av_codec_iterate(&i)) != nullptr) { + if (codec->id != codecId || !av_codec_is_decoder(codec)) { continue; } - const AVCodecHWConfig** config = nullptr; - for (int j = 0; (*config = avcodec_get_hw_config(*codec, j)) != nullptr; + const AVCodecHWConfig* config = nullptr; + for (int j = 0; (config = avcodec_get_hw_config(codec, j)) != nullptr; ++j) { - if ((*config)->device_type == AV_HWDEVICE_TYPE_CUDA) { - return *codec; + if (config->device_type == AV_HWDEVICE_TYPE_CUDA) { + return codec; } } } From 1883af5760bee01789ae8b73e3ead9f583718c33 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Fri, 17 Jan 2025 05:58:23 -0800 Subject: [PATCH 17/17] Move warning and error flags before TORCH flags --- src/torchcodec/decoders/_core/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/torchcodec/decoders/_core/CMakeLists.txt b/src/torchcodec/decoders/_core/CMakeLists.txt index 717e9adf1..7c9a7487a 100644 --- a/src/torchcodec/decoders/_core/CMakeLists.txt +++ b/src/torchcodec/decoders/_core/CMakeLists.txt @@ -4,7 +4,7 @@ set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) find_package(Torch REQUIRED) -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${TORCH_CXX_FLAGS} -Wall -Wextra -pedantic -Werror") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -pedantic -Werror ${TORCH_CXX_FLAGS}") find_package(Python3 ${PYTHON_VERSION} EXACT COMPONENTS Development) function(make_torchcodec_library library_name ffmpeg_target)