diff mbox series

[FFmpeg-devel,1/1] avutil/frame: Document the possibility of negative line sizes

Message ID MN2PR04MB5981A78E7B4B55DA542D5465BAAA9@MN2PR04MB5981.namprd04.prod.outlook.com
State Superseded, archived
Headers show
Series [FFmpeg-devel,1/1] avutil/frame: Document the possibility of negative line sizes
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Soft Works Sept. 30, 2021, 3:30 a.m. UTC
Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavutil/frame.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Soft Works Oct. 11, 2021, 6:08 a.m. UTC | #1
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Soft Works
> Sent: Thursday, September 30, 2021 5:31 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 1/1] avutil/frame: Document the
> possibility of negative line sizes
> 
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>  libavutil/frame.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index ff2540a20f..f07e690410 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -304,7 +304,8 @@ typedef struct AVFrame {
>  #define AV_NUM_DATA_POINTERS 8
>      /**
>       * pointer to the picture/channel planes.
> -     * This might be different from the first allocated byte
> +     * This might be different from the first allocated byte. For
> video,
> +     * it could even point to the end of the image data.
>       *
>       * Some decoders access areas outside 0,0 - width,height, please
>       * see avcodec_align_dimensions2(). Some filters and swscale can
> read
> @@ -313,11 +314,16 @@ typedef struct AVFrame {
>       *
>       * NOTE: Except for hwaccel formats, pointers not needed by the
> format
>       * MUST be set to NULL.
> +     *
> +     * @attention In case of video, the data[] pointers can point to
> the
> +     * end of image data in order to reverse line order, when used
> in
> +     * combination with negative values in the linesize[] array.
>       */
>      uint8_t *data[AV_NUM_DATA_POINTERS];
> 
>      /**
> -     * For video, size in bytes of each picture line.
> +     * For video, a positive or negative value, the ABS() value of
> which is indicating
> +     * the size in bytes of each picture line.
>       * For audio, size in bytes of each plane.
>       *
>       * For audio, only linesize[0] may be set. For planar audio,
> each channel
> @@ -330,6 +336,9 @@ typedef struct AVFrame {
>       *
>       * @note The linesize may be larger than the size of usable data
> -- there
>       * may be extra padding present for performance reasons.
> +     *
> +     * @attention In case of video, line size values can be negative
> to achieve
> +     * a vertically inverted iteration over image lines.
>       */
>      int linesize[AV_NUM_DATA_POINTERS];
> 
> --

Any objections regarding this?

Thanks,
sw
Paul B Mahol Oct. 11, 2021, 4:46 p.m. UTC | #2
lgtm
Michael Niedermayer Oct. 12, 2021, 11:01 a.m. UTC | #3
On Thu, Sep 30, 2021 at 03:30:54AM +0000, Soft Works wrote:
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>  libavutil/frame.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index ff2540a20f..f07e690410 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -304,7 +304,8 @@ typedef struct AVFrame {
>  #define AV_NUM_DATA_POINTERS 8
>      /**
>       * pointer to the picture/channel planes.
> -     * This might be different from the first allocated byte
> +     * This might be different from the first allocated byte. For video,
> +     * it could even point to the end of the image data.
>       *
>       * Some decoders access areas outside 0,0 - width,height, please
>       * see avcodec_align_dimensions2(). Some filters and swscale can read
> @@ -313,11 +314,16 @@ typedef struct AVFrame {
>       *
>       * NOTE: Except for hwaccel formats, pointers not needed by the format
>       * MUST be set to NULL.
> +     *
> +     * @attention In case of video, the data[] pointers can point to the
> +     * end of image data in order to reverse line order, when used in
> +     * combination with negative values in the linesize[] array.
>       */
>      uint8_t *data[AV_NUM_DATA_POINTERS];
>  
>      /**
> -     * For video, size in bytes of each picture line.
> +     * For video, a positive or negative value, the ABS() value of which is indicating
> +     * the size in bytes of each picture line.

from an API point of view linesize really can be anything as long as
1. alignment at the start is legal
2. width*pixelsize is within the array for height times linesize

linesize definitly can be multiple actual lines both positive and negative
so as to acccess even and odd fields of a frame
negative for fliping
but there is more
linesize could be 0 for a more compact representation of a read only
constant color frame.
someone might even point out that one can compress gradients with 
|linesize|<width
so i dont think the ABS() claim here is guranteed to work in every case

i dont think any actual code uses |linesize|<width && linesiez != 0, 
iam not so sure about the linesize = 0 case being used nowhere

[...]

thx
diff mbox series

Patch

diff --git a/libavutil/frame.h b/libavutil/frame.h
index ff2540a20f..f07e690410 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -304,7 +304,8 @@  typedef struct AVFrame {
 #define AV_NUM_DATA_POINTERS 8
     /**
      * pointer to the picture/channel planes.
-     * This might be different from the first allocated byte
+     * This might be different from the first allocated byte. For video,
+     * it could even point to the end of the image data.
      *
      * Some decoders access areas outside 0,0 - width,height, please
      * see avcodec_align_dimensions2(). Some filters and swscale can read
@@ -313,11 +314,16 @@  typedef struct AVFrame {
      *
      * NOTE: Except for hwaccel formats, pointers not needed by the format
      * MUST be set to NULL.
+     *
+     * @attention In case of video, the data[] pointers can point to the
+     * end of image data in order to reverse line order, when used in
+     * combination with negative values in the linesize[] array.
      */
     uint8_t *data[AV_NUM_DATA_POINTERS];
 
     /**
-     * For video, size in bytes of each picture line.
+     * For video, a positive or negative value, the ABS() value of which is indicating
+     * the size in bytes of each picture line.
      * For audio, size in bytes of each plane.
      *
      * For audio, only linesize[0] may be set. For planar audio, each channel
@@ -330,6 +336,9 @@  typedef struct AVFrame {
      *
      * @note The linesize may be larger than the size of usable data -- there
      * may be extra padding present for performance reasons.
+     *
+     * @attention In case of video, line size values can be negative to achieve
+     * a vertically inverted iteration over image lines.
      */
     int linesize[AV_NUM_DATA_POINTERS];