[FFmpeg-devel] [PATCH v5 1/3] lavu/display: Add scaling functions to the display matrix

[Andreas Rheinhardt]

Thilo Borgmann:
> ---
>  doc/APIchanges      |  3 +++
>  libavutil/display.c | 21 +++++++++++++++++++++
>  libavutil/display.h | 15 +++++++++++++++
>  libavutil/version.h |  4 ++--
>  4 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 729f56be7b..42e3391b72 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,9 @@ libavutil:     2021-04-27
>  
>  API changes, most recent first:
>  
> +2022-09-25 - xxxxxxxxxx - lavu 57.37.100 - display.h
> +  Add av_display_matrix_scale(), av_display_scale_get()
> +
>  2022-09-03 - xxxxxxxxxx - lavu 57.36.100 - pixfmt.h
>    Add AV_PIX_FMT_P012, AV_PIX_FMT_Y212, AV_PIX_FMT_XV30, AV_PIX_FMT_XV36
>  
> diff --git a/libavutil/display.c b/libavutil/display.c
> index d31061283c..90758656e3 100644
> --- a/libavutil/display.c
> +++ b/libavutil/display.c
> @@ -48,6 +48,19 @@ double av_display_rotation_get(const int32_t matrix[9])
>      return -rotation;
>  }
>  
> +double av_display_scale_get(const int32_t matrix[9])
> +{
> +    double scale[2];
> +
> +    scale[0] = hypot(CONV_FP(matrix[0]), CONV_FP(matrix[3]));
> +    scale[1] = hypot(CONV_FP(matrix[1]), CONV_FP(matrix[4]));
> +
> +    if (scale[0] == 0.0 || scale[1] == 0.0)
> +        return NAN;
> +
> +    return scale[0];
> +}
> +
>  void av_display_rotation_set(int32_t matrix[9], double angle)
>  {
>      double radians = -angle * M_PI / 180.0f;
> @@ -72,3 +85,11 @@ void av_display_matrix_flip(int32_t matrix[9], int hflip, int vflip)
>          for (i = 0; i < 9; i++)
>              matrix[i] *= flip[i % 3];
>  }
> +
> +void av_display_matrix_scale(int32_t matrix[9], double scale)
> +{
> +    matrix[0] = CONV_DB(CONV_FP(matrix[0]) * scale);
> +    matrix[1] = CONV_DB(CONV_FP(matrix[1]) * scale);
> +    matrix[3] = CONV_DB(CONV_FP(matrix[3]) * scale);
> +    matrix[4] = CONV_DB(CONV_FP(matrix[4]) * scale);
> +}
> diff --git a/libavutil/display.h b/libavutil/display.h
> index 31d8bef361..73767b3a71 100644
> --- a/libavutil/display.h
> +++ b/libavutil/display.h
> @@ -86,6 +86,14 @@
>   */
>  double av_display_rotation_get(const int32_t matrix[9]);
>  
> +/**
> + * Extract the scaling component of the transformation matrix.
> + *
> + * @param matrix the transformation matrix
> + * @return the scaling by which the transformation matrix scales the frame
> + */
> +double av_display_scale_get(const int32_t matrix[9]);

Since when does a matrix have a well-defined scaling component at all?
(Apart from that: I have no clue what relationship the formula in
av_display_scale_get() is supposed to have to scaling. Is it possible
that you forgot that in the vector-matrix multiplication the matrix is
to the right (as opposed to the typical convention in linear algebra)
and that you actually wanted to get the length of the image of the unit
vector in the x direction? If so, you would have still forgotten about
the scaling involving matrix[8] (i.e. w and z in display.h).)

> +
>  /**
>   * Initialize a transformation matrix describing a pure clockwise
>   * rotation by the specified angle (in degrees).
> @@ -105,6 +113,13 @@ void av_display_rotation_set(int32_t matrix[9], double angle);
>   */
>  void av_display_matrix_flip(int32_t matrix[9], int hflip, int vflip);
>  
> +/**
> + * Scale the input matrix.
> + *
> + * @param matrix an allocated transformation matrix

The requirement of the matrix being allocated is bullshit. No need to
copy this.

> + * @param scale scale factor by which the matrix should be scaled
> + */
> +void av_display_matrix_scale(int32_t matrix[9], double scale);
>  /**
>   * @}
>   * @}
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 0585fa7b80..9c44cef6aa 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,8 +79,8 @@
>   */
>  
>  #define LIBAVUTIL_VERSION_MAJOR  57
> -#define LIBAVUTIL_VERSION_MINOR  36
> -#define LIBAVUTIL_VERSION_MICRO 102
> +#define LIBAVUTIL_VERSION_MINOR  37
> +#define LIBAVUTIL_VERSION_MICRO 100
>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>                                                 LIBAVUTIL_VERSION_MINOR, \