diff mbox series

[FFmpeg-devel,1/5] avutil/rational: add av_abs_q

Message ID 20230927100630.50510-2-ffmpeg@haasn.xyz
State New
Headers show
Series work around broken (apple) ICCv4 profiles | expand

Checks

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

Commit Message

Niklas Haas Sept. 27, 2023, 10:03 a.m. UTC
From: Niklas Haas <git@haasn.dev>

Seems like an obvious utility function to be missing, and useful in at
least several places in the code.
---
 doc/APIchanges       |  3 +++
 libavutil/rational.h | 12 ++++++++++++
 libavutil/version.h  |  2 +-
 3 files changed, 16 insertions(+), 1 deletion(-)

Comments

Anton Khirnov Sept. 27, 2023, 10:14 a.m. UTC | #1
Quoting Niklas Haas (2023-09-27 12:03:52)
> +static av_always_inline AVRational av_abs_q(AVRational q)
> +{
> +    if (q.num < 0)
> +        q.num = -q.num;
> +    return q;

What if q.num > 0 and q.den < 0.
Andreas Rheinhardt Sept. 27, 2023, 10:22 a.m. UTC | #2
Niklas Haas:
> From: Niklas Haas <git@haasn.dev>
> 
> Seems like an obvious utility function to be missing, and useful in at
> least several places in the code.
> ---
>  doc/APIchanges       |  3 +++
>  libavutil/rational.h | 12 ++++++++++++
>  libavutil/version.h  |  2 +-
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index f333ff5b24f..2219e32ddc1 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
>  
>  API changes, most recent first:
>  
> +2023-09-27 - xxxxxxxxxx - lavu 58.26.100 - rational.h
> +  Add av_abs_q()
> +
>  2023-09-19 - xxxxxxxxxx - lavu 58.25.100 - avutil.h
>    Make AV_TIME_BASE_Q compatible with C++.
>  
> diff --git a/libavutil/rational.h b/libavutil/rational.h
> index 8cbfc8e0669..7b559ce515d 100644
> --- a/libavutil/rational.h
> +++ b/libavutil/rational.h
> @@ -162,6 +162,18 @@ static av_always_inline AVRational av_inv_q(AVRational q)
>      return r;
>  }
>  
> +/**
> + * Take the absolute value of a rational.
> + * @param q value
> + * @return abs(q)
> + */
> +static av_always_inline AVRational av_abs_q(AVRational q)
> +{
> +    if (q.num < 0)
> +        q.num = -q.num;
> +    return q;
> +}

I agree with Anton: We can't generally assume that the denominator is >
0 and so a generic and safe av_abs_q() will add unnecessary checks in
the cases where it is known that the denominator is positive. In other
words, I don't think this is a good addition to the API.

- Andreas
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index f333ff5b24f..2219e32ddc1 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@  The last version increases of all libraries were on 2023-02-09
 
 API changes, most recent first:
 
+2023-09-27 - xxxxxxxxxx - lavu 58.26.100 - rational.h
+  Add av_abs_q()
+
 2023-09-19 - xxxxxxxxxx - lavu 58.25.100 - avutil.h
   Make AV_TIME_BASE_Q compatible with C++.
 
diff --git a/libavutil/rational.h b/libavutil/rational.h
index 8cbfc8e0669..7b559ce515d 100644
--- a/libavutil/rational.h
+++ b/libavutil/rational.h
@@ -162,6 +162,18 @@  static av_always_inline AVRational av_inv_q(AVRational q)
     return r;
 }
 
+/**
+ * Take the absolute value of a rational.
+ * @param q value
+ * @return abs(q)
+ */
+static av_always_inline AVRational av_abs_q(AVRational q)
+{
+    if (q.num < 0)
+        q.num = -q.num;
+    return q;
+}
+
 /**
  * Convert a double precision floating point number to a rational.
  *
diff --git a/libavutil/version.h b/libavutil/version.h
index 00f229d2335..2dc1a647b8f 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@ 
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR  25
+#define LIBAVUTIL_VERSION_MINOR  26
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \