Skip to content

Commit 19d2c3d

Browse files
committed
Store a UniqueAVFrame in the blocks frame handle
The frame handle held a raw AVFrame*, so _blocks_convert_frame() had to build a UniqueAVFrame around a pointer the handle already owned and then release() it again to undo the fabricated second owner. It only worked because nothing downstream acted on that ownership. Hold a UniqueAVFrame instead. It's an ordinary C++ object whose destructor calls av_frame_free, so the generic wrap_pointer_to_tensor<T> handle used by the other blocks types applies and the bespoke AVFrame wrapper goes away. _blocks_convert_frame() then has a UniqueAVFrame to pass by reference, with no temporary owner in sight. Costs one small allocation per frame for the wrapper object.
1 parent c9e019d commit 19d2c3d

1 file changed

Lines changed: 8 additions & 39 deletions

File tree

src/torchcodec/_core/custom_ops.cpp

Lines changed: 8 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ SingleStreamDecoder* unwrap_tensor_to_get_decoder(
182182
}
183183

184184
// Generic pointer<->tensor laundering for the building-block handle types
185-
// (Demuxer / PacketDecoder / ColorConverter). Same trick as
185+
// (Demuxer / PacketDecoder / ColorConverter / UniqueAVFrame). Same trick as
186186
// wrap_decoder_pointer_to_tensor: the tensor's data pointer IS the raw pointer,
187187
// with a deleter that deletes the owned object when the handle is dropped.
188188
template <typename T>
@@ -206,9 +206,9 @@ T* unwrap_tensor_to_pointer(torch::stable::Tensor& tensor) {
206206
return static_cast<T*>(tensor.mutable_data_ptr());
207207
}
208208

209-
// Opaque packet/frame handles: launder a raw AVPacket*/AVFrame* through a [1]
210-
// int64 CPU tensor whose data pointer IS the raw pointer, with a deleter that
211-
// frees it. Thread-movable, process-local.
209+
// Opaque packet handle: launder a raw AVPacket* through a [1] int64 CPU tensor
210+
// whose data pointer IS the raw pointer, with a deleter that frees it.
211+
// Thread-movable, process-local.
212212
torch::stable::Tensor wrap_packet_pointer_to_tensor(AVPacket* packet) {
213213
auto deleter = [packet](void*) {
214214
AVPacket* p = packet;
@@ -230,32 +230,6 @@ AVPacket* unwrap_tensor_to_packet(torch::stable::Tensor& tensor) {
230230
return static_cast<AVPacket*>(tensor.mutable_data_ptr());
231231
}
232232

233-
torch::stable::Tensor wrap_frame_pointer_to_tensor(AVFrame* frame) {
234-
// Owning handle: the frame is freed when the handle tensor's refcount drops.
235-
// ColorConverter borrows the frame during conversion (on CPU it does not free
236-
// it), so the handle stays the sole owner and there is no leak even if a
237-
// frame is never converted. (GPU conversion would consume the frame; GPU is
238-
// not exposed through these ops yet.)
239-
auto deleter = [frame](void*) {
240-
AVFrame* f = frame;
241-
av_frame_free(&f);
242-
};
243-
int64_t sizes[] = {1};
244-
int64_t strides[] = {1};
245-
return torch::stable::from_blob(
246-
frame,
247-
{sizes, 1},
248-
{strides, 1},
249-
StableDevice(kStableCPU),
250-
kStableInt64,
251-
deleter);
252-
}
253-
254-
AVFrame* unwrap_tensor_to_frame(torch::stable::Tensor& tensor) {
255-
STD_TORCH_CHECK(tensor.is_contiguous(), "frame handle must be contiguous");
256-
return static_cast<AVFrame*>(tensor.mutable_data_ptr());
257-
}
258-
259233
torch::stable::Tensor wrap_multi_stream_encoder_pointer_to_tensor(
260234
std::unique_ptr<MultiStreamEncoder> unique_encoder) {
261235
MultiStreamEncoder* encoder = unique_encoder.release();
@@ -910,9 +884,9 @@ OpsReceiveFrameOutput _blocks_packet_decoder_receive_frame(
910884
AVRational time_base = decoder_ptr->time_base();
911885
double pts_seconds = pts_to_seconds(get_pts_or_dts(av_frame), time_base);
912886
double duration_seconds = pts_to_seconds(get_duration(av_frame), time_base);
913-
AVFrame* raw_frame = av_frame.release();
914887
return std::make_tuple(
915-
wrap_frame_pointer_to_tensor(raw_frame),
888+
wrap_pointer_to_tensor<UniqueAVFrame>(
889+
std::make_unique<UniqueAVFrame>(std::move(av_frame))),
916890
static_cast<int64_t>(0),
917891
pts_seconds,
918892
duration_seconds);
@@ -932,13 +906,8 @@ torch::stable::Tensor _blocks_convert_frame(
932906
torch::stable::Tensor& frame) {
933907
ColorConverter* converter_ptr =
934908
unwrap_tensor_to_pointer<ColorConverter>(converter);
935-
AVFrame* raw_frame = unwrap_tensor_to_frame(frame);
936-
// Borrow the frame for conversion, then release() so the handle keeps
937-
// ownership and frees it when its tensor is dropped (CPU path).
938-
UniqueAVFrame borrowed(raw_frame);
939-
torch::stable::Tensor data = converter_ptr->convert(borrowed);
940-
borrowed.release();
941-
return data;
909+
UniqueAVFrame* frame_ptr = unwrap_tensor_to_pointer<UniqueAVFrame>(frame);
910+
return converter_ptr->convert(*frame_ptr);
942911
}
943912

944913
// For testing only. We need to implement this operation as a core library

0 commit comments

Comments
 (0)