Make C++ compilation strict, treat warnings as errors (#452)

Author: scotts

.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/

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}")
+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)

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<torch::Tensor> preAllocatedOutputTensor) {
+    [[maybe_unused]] const VideoDecoder::VideoStreamDecoderOptions& options,
+    [[maybe_unused]] VideoDecoder::RawDecodedOutput& rawOutput,
+    [[maybe_unused]] VideoDecoder::DecodedOutput& output,
+    [[maybe_unused]] std::optional<torch::Tensor> 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<AVCodecPtr> findCudaCodec(
     const torch::Device& device,
-    const AVCodecID& codecId) {
+    [[maybe_unused]] const AVCodecID& codecId) {
   throwUnsupportedDeviceError(device);
 }
 

src/torchcodec/decoders/_core/CudaDevice.cpp

@@ -9,7 +9,6 @@
 #include "src/torchcodec/decoders/_core/VideoDecoder.h"
 
 extern "C" {
-#include <libavcodec/avcodec.h>
 #include <libavutil/hwcontext_cuda.h>
 #include <libavutil/pixdesc.h>
 }
@@ -110,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;
@@ -260,24 +259,23 @@ 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<AVCodecPtr> findCudaCodec(
+std::optional<const AVCodec*> findCudaCodec(
     const torch::Device& device,
     const AVCodecID& codecId) {
   throwErrorIfNonCudaDevice(device);
 
-  void* i = NULL;
-
-  AVCodecPtr c;
-  while (c = av_codec_iterate(&i)) {
-    const AVCodecHWConfig* config;
-
-    if (c->id != codecId || !av_codec_is_decoder(c)) {
+  void* 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; config = avcodec_get_hw_config(c, j); 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 c;
+        return codec;
       }
     }
   }

src/torchcodec/decoders/_core/DeviceInterface.h

@@ -13,10 +13,6 @@
 #include "FFMPEGCommon.h"
 #include "src/torchcodec/decoders/_core/VideoDecoder.h"
 
-extern "C" {
-#include <libavcodec/avcodec.h>
-}
-
 namespace facebook::torchcodec {
 
 // Note that all these device functions should only be called if the device is

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;
@@ -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<int>(bufferData->size - bufferData->current));
   TORCH_CHECK(
       buf_size >= 0,
       "Tried to read negative bytes: buf_size=",

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<int>(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<int>(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<int>(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<int>(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<int64_t> 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<int64_t>(stream.allFrames.size()),
       "Range stop, " + std::to_string(stop) +
           ", is more than the number of frames, " +
           std::to_string(stream.allFrames.size()));

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

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<int64_t>(streams.size())) {
     throw std::out_of_range(
         "stream_index out of bounds: " + std::to_string(stream_index));
   }