From 760d6daccf5a3e3d291da70b8b568e7ccf231076 Mon Sep 17 00:00:00 2001
From: Nicolas Hug <contact@nicolas-hug.com>
Date: Wed, 22 Jan 2025 18:45:56 +0000
Subject: [PATCH] New names

---
 .../decoders/_core/FFMPEGCommon.cpp           | 12 +++----
 src/torchcodec/decoders/_core/FFMPEGCommon.h  | 35 ++++++++++---------
 .../decoders/_core/VideoDecoder.cpp           |  8 ++---
 3 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/src/torchcodec/decoders/_core/FFMPEGCommon.cpp b/src/torchcodec/decoders/_core/FFMPEGCommon.cpp
index 5cc73592..124e0455 100644
--- a/src/torchcodec/decoders/_core/FFMPEGCommon.cpp
+++ b/src/torchcodec/decoders/_core/FFMPEGCommon.cpp
@@ -10,22 +10,22 @@
 
 namespace facebook::torchcodec {
 
-AVPacketHandler::AVPacketHandler() : avPacket_(av_packet_alloc()) {
+AutoFreedAVPacket::AutoFreedAVPacket() : avPacket_(av_packet_alloc()) {
   TORCH_CHECK(avPacket_ != nullptr, "Couldn't allocate avPacket.");
 }
-AVPacketHandler::~AVPacketHandler() {
+AutoFreedAVPacket::~AutoFreedAVPacket() {
   av_packet_free(&avPacket_);
 }
 
-ReferencedAVPacket::ReferencedAVPacket(AVPacketHandler& shared)
+AutoUnrefedAVPacket::AutoUnrefedAVPacket(AutoFreedAVPacket& shared)
     : avPacket_(shared.avPacket_) {}
-ReferencedAVPacket::~ReferencedAVPacket() {
+AutoUnrefedAVPacket::~AutoUnrefedAVPacket() {
   av_packet_unref(avPacket_);
 }
-AVPacket* ReferencedAVPacket::get() {
+AVPacket* AutoUnrefedAVPacket::get() {
   return avPacket_;
 }
-AVPacket* ReferencedAVPacket::operator->() {
+AVPacket* AutoUnrefedAVPacket::operator->() {
   return avPacket_;
 }
 
diff --git a/src/torchcodec/decoders/_core/FFMPEGCommon.h b/src/torchcodec/decoders/_core/FFMPEGCommon.h
index b82e484b..29833134 100644
--- a/src/torchcodec/decoders/_core/FFMPEGCommon.h
+++ b/src/torchcodec/decoders/_core/FFMPEGCommon.h
@@ -68,39 +68,40 @@ using UniqueAVIOContext = std::
 using UniqueSwsContext =
     std::unique_ptr<SwsContext, Deleter<SwsContext, void, sws_freeContext>>;
 
-// These 2 classes are meant to be used in tandem, like so:
-// ```
-// AVPacketHandler avPacketHandler;
+// These 2 classes share the same underlying AVPacket object. They are meant to
+// be used in tandem, like so:
+//
+// AutoFreedAVPacket autoFreedAVPacket; // <-- malloc for AVPacket happens here
 // while(...){
-//   ReferencedAVPacket packet(avPacketHandler);
-//   av_read_frame(..., packet.get());
-// }
-// ```
+//   AutoUnrefedAVPacket packet(autoFreedAVPacket);
+//   av_read_frame(..., packet.get());  <-- av_packet_ref() called by FFmpeg
+// } <-- av_packet_unref() called here
+//
 // This achieves a few desirable things:
-// - Memory allocation of the underlying AVPacket happens only once, when the
-//   avPacketHandler created.
-// - av_packet_free() is called when avPacketHandler gets out of scope
+// - Memory allocation of the underlying AVPacket happens only once, when
+//   autoFreedAVPacket is created.
+// - av_packet_free() is called when autoFreedAVPacket gets out of scope
 // - av_packet_unref() is automatically called when needed, i.e. at the end of
 //   each loop iteration (or when hitting break / continue). This prevents the
 //   risk of us forgetting to call it.
-class AVPacketHandler {
-  friend class ReferencedAVPacket;
+class AutoFreedAVPacket {
+  friend class AutoUnrefedAVPacket;
 
  private:
   AVPacket* avPacket_;
 
  public:
-  AVPacketHandler();
-  ~AVPacketHandler();
+  AutoFreedAVPacket();
+  ~AutoFreedAVPacket();
 };
 
-class ReferencedAVPacket {
+class AutoUnrefedAVPacket {
  private:
   AVPacket* avPacket_;
 
  public:
-  ReferencedAVPacket(AVPacketHandler& shared);
-  ~ReferencedAVPacket();
+  AutoUnrefedAVPacket(AutoFreedAVPacket& shared);
+  ~AutoUnrefedAVPacket();
   AVPacket* get();
   AVPacket* operator->();
 };
diff --git a/src/torchcodec/decoders/_core/VideoDecoder.cpp b/src/torchcodec/decoders/_core/VideoDecoder.cpp
index 92be44fc..d81ebec4 100644
--- a/src/torchcodec/decoders/_core/VideoDecoder.cpp
+++ b/src/torchcodec/decoders/_core/VideoDecoder.cpp
@@ -573,9 +573,9 @@ void VideoDecoder::scanFileAndUpdateMetadataAndIndex() {
     return;
   }
 
-  AVPacketHandler avPacketHandler;
+  AutoFreedAVPacket autoFreedAVPacket;
   while (true) {
-    ReferencedAVPacket packet(avPacketHandler);
+    AutoUnrefedAVPacket packet(autoFreedAVPacket);
 
     // av_read_frame is a misleading name: it gets the next **packet**.
     int ffmpegStatus = av_read_frame(formatContext_.get(), packet.get());
@@ -806,7 +806,7 @@ VideoDecoder::RawDecodedOutput VideoDecoder::getDecodedOutputWithFilter(
   }
   // Need to get the next frame or error from PopFrame.
   UniqueAVFrame frame(av_frame_alloc());
-  AVPacketHandler avPacketHandler;
+  AutoFreedAVPacket autoFreedAVPacket;
   int ffmpegStatus = AVSUCCESS;
   bool reachedEOF = false;
   int frameStreamIndex = -1;
@@ -846,7 +846,7 @@ VideoDecoder::RawDecodedOutput VideoDecoder::getDecodedOutputWithFilter(
       // pulling frames from its internal buffers.
       continue;
     }
-    ReferencedAVPacket packet(avPacketHandler);
+    AutoUnrefedAVPacket packet(autoFreedAVPacket);
     ffmpegStatus = av_read_frame(formatContext_.get(), packet.get());
     decodeStats_.numPacketsRead++;
     if (ffmpegStatus == AVERROR_EOF) {