Re: [FFmpeg-devel] [PATCH 04/17] swscale/graph: move vshift() and shift_img() to shared header

[Ramiro Polla <ramiro.polla@gmail.com>]

On Sat, Apr 26, 2025 at 7:57 PM Niklas Haas <ffmpeg@haasn.xyz> wrote:
>
> From: Niklas Haas <git@haasn.dev>
>
> I need to reuse these inside `ops.c`.
> ---
>  libswscale/graph.c | 29 +++++++----------------------
>  libswscale/graph.h | 13 +++++++++++++
>  2 files changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/libswscale/graph.c b/libswscale/graph.c
> index c5a46eb257..b921b7ec02 100644
> --- a/libswscale/graph.c
> +++ b/libswscale/graph.c
> @@ -94,29 +94,14 @@ static int pass_append(SwsGraph *graph, enum AVPixelFormat fmt, int w, int h,
>      return 0;
>  }
>
> -static int vshift(enum AVPixelFormat fmt, int plane)
> -{
> -    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(fmt);
> -    return (plane == 1 || plane == 2) ? desc->log2_chroma_h : 0;
> -}
> -
> -/* Shift an image vertically by y lines */
> -static SwsImg shift_img(const SwsImg *img_base, int y)
> -{
> -    SwsImg img = *img_base;
> -    for (int i = 0; i < 4 && img.data[i]; i++)
> -        img.data[i] += (y >> vshift(img.fmt, i)) * img.linesize[i];
> -    return img;
> -}
> -
>  static void run_copy(const SwsImg *out_base, const SwsImg *in_base,
>                       int y, int h, const SwsPass *pass)
>  {
> -    SwsImg in  = shift_img(in_base,  y);
> -    SwsImg out = shift_img(out_base, y);
> +    SwsImg in  = ff_sws_img_shift(*in_base,  y);
> +    SwsImg out = ff_sws_img_shift(*out_base, y);
>
>      for (int i = 0; i < FF_ARRAY_ELEMS(out.data) && out.data[i]; i++) {
> -        const int lines = h >> vshift(in.fmt, i);
> +        const int lines = h >> ff_fmt_vshift(in.fmt, i);
>          av_assert1(in.data[i]);
>
>          if (in.linesize[i] == out.linesize[i]) {
[...]
> diff --git a/libswscale/graph.h b/libswscale/graph.h
> index 62b622a065..191734b794 100644
> --- a/libswscale/graph.h
> +++ b/libswscale/graph.h
> @@ -34,6 +34,19 @@ typedef struct SwsImg {
>      int linesize[4];
>  } SwsImg;
>
> +static av_always_inline av_const int ff_fmt_vshift(enum AVPixelFormat fmt, int plane)
> +{
> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(fmt);
> +    return (plane == 1 || plane == 2) ? desc->log2_chroma_h : 0;
> +}
> +
> +static av_const inline SwsImg ff_sws_img_shift(SwsImg img, const int y)
> +{
> +    for (int i = 0; i < 4 && img.data[i]; i++)
> +        img.data[i] += (y >> ff_fmt_vshift(img.fmt, i)) * img.linesize[i];
> +    return img;
> +}

I find it weird to pass the struct itself as an argument. The previous
version took a const * and made the copy itself. The new version uses
a dereference to make a copy at the call site. The compiler probably
optimizes both down to the same binary, since the function is inline.

Either are fine by me btw, I just wanted to point it out.
_______________________________________________
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".