diff --git a/av/video/frame.py b/av/video/frame.py index b1bdcf8e2..cbf541c62 100644 --- a/av/video/frame.py +++ b/av/video/frame.py @@ -456,6 +456,20 @@ def alloc_video_frame() -> VideoFrame: return VideoFrame(_cinit_bypass_sentinel) +def _check_crop(value, other, size, names): + # Mirror av_frame_apply_cropping(): the two crops on an axis must leave at + # least one pixel. Some hardware filters read these fields unchecked. + value = int(value) + if value < 0: + raise ValueError(f"{names[0]} must not be negative") + if value and value + other >= size: + raise ValueError( + f"{names[0]} + {names[1]} must be less than the frame " + f"{names[2]} ({size}), got {value} + {other}" + ) + return value + + class PictureType(IntEnum): NONE = lib.AV_PICTURE_TYPE_NONE # Undefined I = lib.AV_PICTURE_TYPE_I # Intra @@ -722,6 +736,10 @@ def crop_top(self): option is ``"0"``, in which case the crop is reported here and can be applied with :meth:`apply_cropping`. + Setting a value raises :class:`ValueError` if it would leave no rows, + i.e. if ``crop_top + crop_bottom`` would reach :attr:`height`. The same + goes for ``crop_left + crop_right`` and :attr:`width`. + Wraps :ffmpeg:`AVFrame.crop_top`. :type: int @@ -730,7 +748,12 @@ def crop_top(self): @crop_top.setter def crop_top(self, value): - self.ptr.crop_top = value + self.ptr.crop_top = _check_crop( + value, + self.ptr.crop_bottom, + self.ptr.height, + ("crop_top", "crop_bottom", "height"), + ) @property def crop_bottom(self): @@ -744,7 +767,12 @@ def crop_bottom(self): @crop_bottom.setter def crop_bottom(self, value): - self.ptr.crop_bottom = value + self.ptr.crop_bottom = _check_crop( + value, + self.ptr.crop_top, + self.ptr.height, + ("crop_bottom", "crop_top", "height"), + ) @property def crop_left(self): @@ -758,7 +786,12 @@ def crop_left(self): @crop_left.setter def crop_left(self, value): - self.ptr.crop_left = value + self.ptr.crop_left = _check_crop( + value, + self.ptr.crop_right, + self.ptr.width, + ("crop_left", "crop_right", "width"), + ) @property def crop_right(self): @@ -772,7 +805,12 @@ def crop_right(self): @crop_right.setter def crop_right(self, value): - self.ptr.crop_right = value + self.ptr.crop_right = _check_crop( + value, + self.ptr.crop_left, + self.ptr.width, + ("crop_right", "crop_left", "width"), + ) def apply_cropping(self, unaligned=False): """Crop the frame in place according to its ``crop_*`` attributes. @@ -780,6 +818,10 @@ def apply_cropping(self, unaligned=False): No pixels are copied; the planes are narrowed to the cropped region and the ``crop_*`` attributes are reset to 0. + For hardware frames and 1-bit formats (``monob``, ``monow``), FFmpeg only + applies :attr:`crop_right` and :attr:`crop_bottom`; :attr:`crop_left` and + :attr:`crop_top` are left unapplied and keep their values. + By default the left edge may be moved slightly to keep plane data aligned, leaving a few extra pixels. Pass ``unaligned=True`` to crop exactly. diff --git a/av/video/plane.pxd b/av/video/plane.pxd index 80ff79050..72112a771 100644 --- a/av/video/plane.pxd +++ b/av/video/plane.pxd @@ -5,8 +5,13 @@ from av.video.format cimport VideoFormatComponent cdef class VideoPlane(Plane): - cdef readonly size_t buffer_size - cdef readonly unsigned int width, height + # Geometry derived from the frame, recomputed by _refresh() whenever the + # frame's dimensions or data pointer change (e.g. after apply_cropping()). + cdef size_t _size + cdef unsigned int _width, _height + cdef int _seen_width, _seen_height, _seen_linesize + cdef void *_seen_data + cdef void _refresh(self) cdef enum DeviceType: diff --git a/av/video/plane.py b/av/video/plane.py index 2d056103f..0df7f643b 100644 --- a/av/video/plane.py +++ b/av/video/plane.py @@ -21,6 +21,23 @@ @cython.cclass class VideoPlane(Plane): def __cinit__(self, frame: VideoFrame, index: cython.int): + self._seen_data = cython.NULL + self._refresh() + + @cython.cfunc + def _refresh(self) -> cython.void: + frame: VideoFrame = self.frame + index: cython.int = self.index + linesize: cython.int = frame.ptr.linesize[index] + data: cython.p_void = frame.ptr.data[index] + if ( + self._seen_data == data + and self._seen_width == frame.ptr.width + and self._seen_height == frame.ptr.height + and self._seen_linesize == linesize + ): + return + # The palette plane has no associated component or linesize; set fields manually fmt = frame.format if frame.ptr.hw_frames_ctx: @@ -32,44 +49,70 @@ def __cinit__(self, frame: VideoFrame, index: cython.int): ) if index == 1 and fmt.name == "pal8": - self.width = 256 - self.height = 1 - self.buffer_size = 256 * 4 - return - - for i in range(fmt.ptr.nb_components): - if fmt.ptr.comp[i].plane == index: - component = VideoFormatComponent(fmt, i) - self.width = component.width - self.height = component.height - break + self._width = 256 + self._height = 1 + self._size = 256 * 4 else: - raise RuntimeError(f"could not find plane {index} of {fmt!r}") - - # Sometimes, linesize is negative (and that is meaningful). We are only - # insisting that the buffer size be based on the extent of linesize, and - # ignore it's direction. - self.buffer_size = abs(self.frame.ptr.linesize[self.index]) * self.height - - # Cropping moves the data pointer into the buffer, so the last row can end - # past the allocation. Clamp to what the plane's buffer actually holds. - if frame.ptr.hw_frames_ctx or self.frame.ptr.linesize[self.index] < 0: - return - buf: cython.pointer[lib.AVBufferRef] = lib.av_frame_get_plane_buffer( - frame.ptr, index - ) - if buf == cython.NULL: - return - start: cython.p_uchar = frame.ptr.data[index] - end: cython.p_uchar = buf.data + buf.size - if buf.data <= start < end and cython.cast(cython.size_t, end - start) < ( - self.buffer_size - ): - self.buffer_size = end - start + for i in range(fmt.ptr.nb_components): + if fmt.ptr.comp[i].plane == index: + component = VideoFormatComponent(fmt, i) + self._width = component.width + self._height = component.height + break + else: + raise RuntimeError(f"could not find plane {index} of {fmt!r}") + + # Sometimes, linesize is negative (and that is meaningful). We are only + # insisting that the buffer size be based on the extent of linesize, and + # ignore it's direction. + self._size = abs(linesize) * self._height + + # Cropping moves the data pointer into the buffer, so the last row can + # end past the allocation. Clamp to what the plane's buffer holds. + if not frame.ptr.hw_frames_ctx and linesize >= 0: + buf: cython.pointer[lib.AVBufferRef] = lib.av_frame_get_plane_buffer( + frame.ptr, index + ) + if buf != cython.NULL: + start: cython.p_uchar = frame.ptr.data[index] + end: cython.p_uchar = buf.data + buf.size + if ( + buf.data <= start < end + and cython.cast(cython.size_t, end - start) < self._size + ): + self._size = end - start + + self._seen_data = data + self._seen_width = frame.ptr.width + self._seen_height = frame.ptr.height + self._seen_linesize = linesize @cython.cfunc def _buffer_size(self) -> cython.size_t: - return self.buffer_size + self._refresh() + return self._size + + @property + def buffer_size(self): + return self._buffer_size() + + @property + def width(self): + """Width of this plane, in pixels. + + :type: int + """ + self._refresh() + return self._width + + @property + def height(self): + """Height of this plane, in pixels. + + :type: int + """ + self._refresh() + return self._height @property def line_size(self): @@ -95,13 +138,14 @@ def __getbuffer__(self, view: cython.pointer[Py_buffer], flags: cython.int): if flags & PyBUF_WRITABLE and not self._buffer_writable(): raise ValueError("buffer is not writable") + size: cython.size_t = self._buffer_size() ptr: cython.p_void = self._buffer_ptr() line_size: cython.int = self.frame.ptr.linesize[self.index] if line_size < 0: - height: cython.int = self.height + height: cython.int = self._height ptr = cython.cast(cython.p_char, ptr) + (height - 1) * line_size - PyBuffer_FillInfo(view, self, ptr, self._buffer_size(), 0, flags) + PyBuffer_FillInfo(view, self, ptr, size, 0, flags) def __dlpack_device__(self): if self.frame.ptr.hw_frames_ctx: @@ -148,6 +192,7 @@ def __dlpack__(self, *, stream: int | None = None): device_type = kCPU device_id = 0 + self._refresh() line_size = self.line_size if line_size < 0: raise NotImplementedError( @@ -260,14 +305,14 @@ def __dlpack__(self, *, stream: int | None = None): if ncomp == 1: ndim = 2 - s0 = self.height - s1 = self.width + s0 = self._height + s1 = self._width st0 = line_size // itemsize st1 = 1 else: ndim = 3 - s0 = self.height - s1 = self.width + s0 = self._height + s1 = self._width s2 = ncomp st0 = line_size // itemsize st1 = ncomp diff --git a/av/video/reformatter.py b/av/video/reformatter.py index 7498bdcf1..030fc69fb 100644 --- a/av/video/reformatter.py +++ b/av/video/reformatter.py @@ -303,6 +303,15 @@ def _reformat( new_frame.ptr.width = width new_frame.ptr.height = height + # The copied crop is in source pixels. It still holds after a pixel + # format change, but not after a resize, where it could even exceed the + # new dimensions. + if width != frame.ptr.width or height != frame.ptr.height: + new_frame.ptr.crop_top = 0 + new_frame.ptr.crop_bottom = 0 + new_frame.ptr.crop_left = 0 + new_frame.ptr.crop_right = 0 + # A transfer-characteristic / primaries conversion is opt-in. Unlike the # pre-17.0 sws_scale, sws_scale_frame inspects color_trc/color_primaries # and rejects RESERVED (and other unsupported) values with EOPNOTSUPP, diff --git a/tests/test_videoframe.py b/tests/test_videoframe.py index 284ae32f0..bc75d7a0d 100644 --- a/tests/test_videoframe.py +++ b/tests/test_videoframe.py @@ -1582,11 +1582,29 @@ def test_apply_cropping_plane_stays_in_buffer() -> None: assertNdarraysEqual(frame.to_ndarray(), array[:, 10:]) -def test_apply_cropping_invalid() -> None: +def test_crop_setters_reject_invalid() -> None: frame = VideoFrame(64, 48, "yuv420p") - frame.crop_left = 100 - with pytest.raises(av.error.OverflowError): - frame.apply_cropping() + with pytest.raises(ValueError): + frame.crop_left = 64 + with pytest.raises(ValueError): + frame.crop_top = -1 + + frame.crop_left = 40 + with pytest.raises(ValueError, match="crop_right"): + frame.crop_right = 24 + frame.crop_right = 23 + + frame.crop_bottom = 47 + with pytest.raises(ValueError, match="crop_top"): + frame.crop_top = 1 + assert (frame.crop_left, frame.crop_right) == (40, 23) + assert (frame.crop_top, frame.crop_bottom) == (0, 47) + + # Zero is always allowed, even on a frame with no pixels. + empty = VideoFrame() + empty.crop_left = 0 + with pytest.raises(ValueError): + empty.crop_left = 1 def _encode_m2v() -> bytearray: @@ -1645,3 +1663,58 @@ def test_decode_error_flags() -> None: frames = _decode_m2v(data) assert any(DecodeError.concealment_active in f.decode_error_flags for f in frames) assert any(frame.is_corrupt for frame in frames) + + +def test_apply_cropping_updates_existing_planes() -> None: + array = numpy.arange(48 * 64, dtype=numpy.uint8).reshape(48, 64) + frame = VideoFrame.from_numpy_buffer(array, format="gray") + plane = frame.planes[0] + frame.crop_top = 40 + frame.crop_left = 8 + frame.apply_cropping(unaligned=True) + + # A plane made before cropping must not pair the shifted data pointer with + # its old size, or reading it runs past the end of the array. + assert (plane.width, plane.height) == (56, 8) + assert plane.buffer_size == 64 * 7 + 56 + assert bytes(plane) == array.tobytes()[40 * 64 + 8 :] + + +@pytest.mark.parametrize("format", ["monob", "monow"]) +def test_apply_cropping_bitstream(format: str) -> None: + frame = VideoFrame(64, 48, format) + plane = frame.planes[0] + line_size = plane.line_size + frame.crop_left = 8 + frame.crop_top = 4 + frame.crop_right = 3 + frame.crop_bottom = 5 + frame.apply_cropping() + + # FFmpeg cannot move a data pointer by a fraction of a byte, so only the + # right and bottom crops apply. + assert (frame.width, frame.height) == (61, 43) + assert (frame.crop_left, frame.crop_top) == (8, 4) + assert (frame.crop_right, frame.crop_bottom) == (0, 0) + assert plane.buffer_size == line_size * 43 + assert frame.to_ndarray(format="rgb24").shape == (43, 61, 3) + + +def test_reformat_crop() -> None: + frame = VideoFrame(64, 48, "yuv420p") + frame.crop_left = 40 + frame.crop_top = 30 + frame.crop_bottom = 2 + + # Same size: the crop still describes the same pixels. + rgb = frame.reformat(format="rgb24") + assert (rgb.crop_left, rgb.crop_top, rgb.crop_bottom) == (40, 30, 2) + rgb.apply_cropping(unaligned=True) + assert (rgb.width, rgb.height) == (24, 16) + + # Resized: the source crop no longer fits, so it is dropped. + small = frame.reformat(width=32, height=24, format="rgb24") + assert small.crop_left == small.crop_top == small.crop_bottom == 0 + assert small.crop_right == 0 + small.apply_cropping() + assert (small.width, small.height) == (32, 24)