Skip to content

Commit 9a9d17c

Browse files
authored
Move packet allocation out of decoding loop (#461)
1 parent 0ad3f01 commit 9a9d17c

File tree

3 files changed

+64
-6
lines changed

3 files changed

+64
-6
lines changed

src/torchcodec/decoders/_core/FFMPEGCommon.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,25 @@
1010

1111
namespace facebook::torchcodec {
1212

13+
AutoAVPacket::AutoAVPacket() : avPacket_(av_packet_alloc()) {
14+
TORCH_CHECK(avPacket_ != nullptr, "Couldn't allocate avPacket.");
15+
}
16+
AutoAVPacket::~AutoAVPacket() {
17+
av_packet_free(&avPacket_);
18+
}
19+
20+
ReferenceAVPacket::ReferenceAVPacket(AutoAVPacket& shared)
21+
: avPacket_(shared.avPacket_) {}
22+
ReferenceAVPacket::~ReferenceAVPacket() {
23+
av_packet_unref(avPacket_);
24+
}
25+
AVPacket* ReferenceAVPacket::get() {
26+
return avPacket_;
27+
}
28+
AVPacket* ReferenceAVPacket::operator->() {
29+
return avPacket_;
30+
}
31+
1332
AVCodecOnlyUseForCallingAVFindBestStream
1433
makeAVCodecOnlyUseForCallingAVFindBestStream(const AVCodec* codec) {
1534
#if LIBAVCODEC_VERSION_INT < AV_VERSION_INT(59, 18, 100)

src/torchcodec/decoders/_core/FFMPEGCommon.h

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,6 @@ using UniqueAVCodecContext = std::unique_ptr<
5757
Deleterp<AVCodecContext, void, avcodec_free_context>>;
5858
using UniqueAVFrame =
5959
std::unique_ptr<AVFrame, Deleterp<AVFrame, void, av_frame_free>>;
60-
using UniqueAVPacket =
61-
std::unique_ptr<AVPacket, Deleterp<AVPacket, void, av_packet_free>>;
6260
using UniqueAVFilterGraph = std::unique_ptr<
6361
AVFilterGraph,
6462
Deleterp<AVFilterGraph, void, avfilter_graph_free>>;
@@ -70,6 +68,44 @@ using UniqueAVIOContext = std::
7068
using UniqueSwsContext =
7169
std::unique_ptr<SwsContext, Deleter<SwsContext, void, sws_freeContext>>;
7270

71+
// These 2 classes share the same underlying AVPacket object. They are meant to
72+
// be used in tandem, like so:
73+
//
74+
// AutoAVPacket autoAVPacket; // <-- malloc for AVPacket happens here
75+
// while(...){
76+
// ReferenceAVPacket packet(autoAVPacket);
77+
// av_read_frame(..., packet.get()); <-- av_packet_ref() called by FFmpeg
78+
// } <-- av_packet_unref() called here
79+
//
80+
// This achieves a few desirable things:
81+
// - Memory allocation of the underlying AVPacket happens only once, when
82+
// autoAVPacket is created.
83+
// - av_packet_free() is called when autoAVPacket gets out of scope
84+
// - av_packet_unref() is automatically called when needed, i.e. at the end of
85+
// each loop iteration (or when hitting break / continue). This prevents the
86+
// risk of us forgetting to call it.
87+
class AutoAVPacket {
88+
friend class ReferenceAVPacket;
89+
90+
private:
91+
AVPacket* avPacket_;
92+
93+
public:
94+
AutoAVPacket();
95+
~AutoAVPacket();
96+
};
97+
98+
class ReferenceAVPacket {
99+
private:
100+
AVPacket* avPacket_;
101+
102+
public:
103+
ReferenceAVPacket(AutoAVPacket& shared);
104+
~ReferenceAVPacket();
105+
AVPacket* get();
106+
AVPacket* operator->();
107+
};
108+
73109
// av_find_best_stream is not const-correct before commit:
74110
// https://github.com/FFmpeg/FFmpeg/commit/46dac8cf3d250184ab4247809bc03f60e14f4c0c
75111
// which was released in FFMPEG version=5.0.3

src/torchcodec/decoders/_core/VideoDecoder.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -573,9 +573,11 @@ void VideoDecoder::scanFileAndUpdateMetadataAndIndex() {
573573
return;
574574
}
575575

576+
AutoAVPacket autoAVPacket;
576577
while (true) {
577-
// Get the next packet.
578-
UniqueAVPacket packet(av_packet_alloc());
578+
ReferenceAVPacket packet(autoAVPacket);
579+
580+
// av_read_frame is a misleading name: it gets the next **packet**.
579581
int ffmpegStatus = av_read_frame(formatContext_.get(), packet.get());
580582

581583
if (ffmpegStatus == AVERROR_EOF) {
@@ -804,6 +806,7 @@ VideoDecoder::RawDecodedOutput VideoDecoder::getDecodedOutputWithFilter(
804806
}
805807
// Need to get the next frame or error from PopFrame.
806808
UniqueAVFrame frame(av_frame_alloc());
809+
AutoAVPacket autoAVPacket;
807810
int ffmpegStatus = AVSUCCESS;
808811
bool reachedEOF = false;
809812
int frameStreamIndex = -1;
@@ -843,7 +846,7 @@ VideoDecoder::RawDecodedOutput VideoDecoder::getDecodedOutputWithFilter(
843846
// pulling frames from its internal buffers.
844847
continue;
845848
}
846-
UniqueAVPacket packet(av_packet_alloc());
849+
ReferenceAVPacket packet(autoAVPacket);
847850
ffmpegStatus = av_read_frame(formatContext_.get(), packet.get());
848851
decodeStats_.numPacketsRead++;
849852
if (ffmpegStatus == AVERROR_EOF) {
@@ -874,12 +877,12 @@ VideoDecoder::RawDecodedOutput VideoDecoder::getDecodedOutputWithFilter(
874877
}
875878
ffmpegStatus = avcodec_send_packet(
876879
streams_[packet->stream_index].codecContext.get(), packet.get());
877-
decodeStats_.numPacketsSentToDecoder++;
878880
if (ffmpegStatus < AVSUCCESS) {
879881
throw std::runtime_error(
880882
"Could not push packet to decoder: " +
881883
getFFMPEGErrorStringFromErrorCode(ffmpegStatus));
882884
}
885+
decodeStats_.numPacketsSentToDecoder++;
883886
}
884887
if (ffmpegStatus < AVSUCCESS) {
885888
if (reachedEOF || ffmpegStatus == AVERROR_EOF) {

0 commit comments

Comments
 (0)