Skip to content

Commit 43d6dde

Browse files
committed
AVIOFileLikeContext refactoring
1 parent 681b9cc commit 43d6dde

File tree

1 file changed

+18
-22
lines changed

1 file changed

+18
-22
lines changed

src/torchcodec/decoders/_core/PyBindOps.cpp

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,12 @@ struct PyObjectDeleter {
2626
}
2727
};
2828

29+
using UniquePyObject = std::unique_ptr<py::object, PyObjectDeleter>;
30+
2931
class AVIOFileLikeContext : public AVIOContextHolder {
3032
public:
3133
explicit AVIOFileLikeContext(py::object fileLike)
32-
: fileLikeContext_{std::unique_ptr<py::object, PyObjectDeleter>(
33-
new py::object(fileLike))} {
34+
: fileLike_{UniquePyObject(new py::object(fileLike))} {
3435
{
3536
// TODO: Is it necessary to acquire the GIL here? Is it maybe even
3637
// harmful? At the moment, this is only called from within a pybind
@@ -43,18 +44,18 @@ class AVIOFileLikeContext : public AVIOContextHolder {
4344
py::hasattr(fileLike, "seek"),
4445
"File like object must implement a seek method.");
4546
}
46-
createAVIOContext(&read, &seek, &fileLikeContext_);
47+
createAVIOContext(&read, &seek, &fileLike_);
4748
}
4849

4950
static int read(void* opaque, uint8_t* buf, int buf_size) {
50-
auto fileLikeContext = static_cast<FileLikeContext*>(opaque);
51+
auto fileLike = static_cast<UniquePyObject*>(opaque);
5152

5253
int num_read = 0;
5354
while (num_read < buf_size) {
5455
int request = buf_size - num_read;
5556
py::gil_scoped_acquire gil;
56-
auto chunk = static_cast<std::string>(static_cast<py::bytes>(
57-
fileLikeContext->fileLike->attr("read")(request)));
57+
auto chunk = static_cast<std::string>(
58+
static_cast<py::bytes>((*fileLike)->attr("read")(request)));
5859
int chunk_len = static_cast<int>(chunk.length());
5960
if (chunk_len == 0) {
6061
break;
@@ -78,26 +79,21 @@ class AVIOFileLikeContext : public AVIOContextHolder {
7879
if (whence == AVSEEK_SIZE) {
7980
return AVERROR(EIO);
8081
}
81-
auto fileLikeContext = static_cast<FileLikeContext*>(opaque);
82+
auto fileLike = static_cast<UniquePyObject*>(opaque);
8283
py::gil_scoped_acquire gil;
83-
return py::cast<int64_t>(
84-
fileLikeContext->fileLike->attr("seek")(offset, whence));
84+
return py::cast<int64_t>((*fileLike)->attr("seek")(offset, whence));
8585
}
8686

8787
private:
88-
struct FileLikeContext {
89-
// Note that we keep a pointer to the Python object because we need to
90-
// strictly control when its destructor is called. We must hold the GIL
91-
// when its destructor gets called, as it needs to update the reference
92-
// count. It's easiest to control that when it's a pointer. Otherwise, we'd
93-
// have to ensure whatever enclosing scope holds the object has the GIL,
94-
// and that's, at least, hard. For all of the common pitfalls, see:
95-
//
96-
// https://pybind11.readthedocs.io/en/stable/advanced/misc.html#common-sources-of-global-interpreter-lock-errors
97-
std::unique_ptr<py::object, PyObjectDeleter> fileLike;
98-
};
99-
100-
FileLikeContext fileLikeContext_;
88+
// Note that we keep a pointer to the Python object because we need to
89+
// strictly control when its destructor is called. We must hold the GIL
90+
// when its destructor gets called, as it needs to update the reference
91+
// count. It's easiest to control that when it's a pointer. Otherwise, we'd
92+
// have to ensure whatever enclosing scope holds the object has the GIL,
93+
// and that's, at least, hard. For all of the common pitfalls, see:
94+
//
95+
// https://pybind11.readthedocs.io/en/stable/advanced/misc.html#common-sources-of-global-interpreter-lock-errors
96+
UniquePyObject fileLike_;
10197
};
10298

10399
} // namespace

0 commit comments

Comments
 (0)