Skip to content

Commit

Permalink
Guard input container properties and methods again use after close
Browse files Browse the repository at this point in the history
Calling `close()` on an input container calls `avformat_close_input`
which will set the `self.ptr` AVFormatContext pointer to NULL. We need
to ensure `self.ptr` is not NULL before using it.

Fixes: #1137
  • Loading branch information
jlaine committed Nov 1, 2023
1 parent 24bcd1f commit 0c8ade3
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 2 deletions.
2 changes: 2 additions & 0 deletions av/container/core.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ cdef class Container:
cdef readonly StreamContainer streams
cdef readonly dict metadata

# Private API.
cdef _assert_open(self)
cdef int err_check(self, int value) except -1

# Timeouts
Expand Down
7 changes: 7 additions & 0 deletions av/container/core.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ cdef class Container:
return err_check(value, filename=self.name)

def dumps_format(self):
self._assert_open()
with LogCapture() as logs:
lib.av_dump_format(self.ptr, 0, "", isinstance(self, OutputContainer))
return ''.join(log[2] for log in logs)
Expand All @@ -298,10 +299,16 @@ cdef class Container:
cdef start_timeout(self):
self.interrupt_callback_info.start_time = clock()

cdef _assert_open(self):
if self.ptr == NULL:
raise AssertionError("Container is not open")

def _get_flags(self):
self._assert_open()
return self.ptr.flags

def _set_flags(self, value):
self._assert_open()
self.ptr.flags = value

flags = Flags.property(
Expand Down
16 changes: 14 additions & 2 deletions av/container/input.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ from av.dictionary import Dictionary
cdef close_input(InputContainer self):
if self.input_was_opened:
with nogil:
# This causes `self.ptr` to be set to NULL.
lib.avformat_close_input(&self.ptr)
self.input_was_opened = False

Expand Down Expand Up @@ -90,19 +91,25 @@ cdef class InputContainer(Container):

property start_time:
def __get__(self):
self._assert_open()
if self.ptr.start_time != lib.AV_NOPTS_VALUE:
return self.ptr.start_time

property duration:
def __get__(self):
self._assert_open()
if self.ptr.duration != lib.AV_NOPTS_VALUE:
return self.ptr.duration

property bit_rate:
def __get__(self): return self.ptr.bit_rate
def __get__(self):
self._assert_open()
return self.ptr.bit_rate

property size:
def __get__(self): return lib.avio_size(self.ptr.pb)
def __get__(self):
self._assert_open()
return lib.avio_size(self.ptr.pb)

def close(self):
close_input(self)
Expand All @@ -123,6 +130,7 @@ cdef class InputContainer(Container):
.. note:: The last packets are dummy packets that when decoded will flush the buffers.
"""
self._assert_open()

# For whatever reason, Cython does not like us directly passing kwargs
# from one method to another. Without kwargs, it ends up passing a
Expand Down Expand Up @@ -198,6 +206,7 @@ cdef class InputContainer(Container):
the arguments.
"""
self._assert_open()
id(kwargs) # Avoid Cython bug; see demux().
for packet in self.demux(*args, **kwargs):
for frame in packet.decode():
Expand Down Expand Up @@ -234,6 +243,7 @@ cdef class InputContainer(Container):
.. seealso:: :ffmpeg:`avformat_seek_file` for discussion of the flags.
"""
self._assert_open()

# We used to take floats here and assume they were in seconds. This
# was super confusing, so lets go in the complete opposite direction
Expand Down Expand Up @@ -270,6 +280,8 @@ cdef class InputContainer(Container):
self.flush_buffers()

cdef flush_buffers(self):
self._assert_open()

cdef Stream stream
cdef CodecContext codec_context

Expand Down
13 changes: 13 additions & 0 deletions tests/test_decode.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,16 @@ def test_decode_video_corrupt(self):

self.assertEqual(packet_count, 1)
self.assertEqual(frame_count, 0)

def test_decode_close_then_use(self):
container = av.open(fate_suite("h264/interlaced_crop.mp4"))
container.close()

# Check accessing every attribute either works or raises
# an `AssertionError`.
for attr in dir(container):
with self.subTest(attr=attr):
try:
getattr(container, attr)
except AssertionError:
pass

0 comments on commit 0c8ade3

Please sign in to comment.