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

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".