[FFmpeg-devel] [PATCH 1/4] avfilter/x86/vf_blend: use unaligned movs for output

[FFmpeg-devel] [PATCH 1/4] avfilter/x86/vf_blend: use unaligned movs for output

[Marton Balint]

Fixes crashes with:

ffmpeg -f lavfi -i allyuv=d=1 -vf tblend=difference128,pad=5000:ih:1 -f null x

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/x86/vf_blend.asm | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libavfilter/x86/vf_blend.asm b/libavfilter/x86/vf_blend.asm
index 766e5b7bc1..277b100e4d 100644
--- a/libavfilter/x86/vf_blend.asm
+++ b/libavfilter/x86/vf_blend.asm
@@ -75,7 +75,7 @@ BLEND_INIT %1, 2, %3
         movu            m0, [topq + xq]
         movu            m1, [bottomq + xq]
         p%2             m0, m1
-        mova   [dstq + xq], m0
+        movu   [dstq + xq], m0
         add             xq, mmsize
     jl .loop
 BLEND_END
@@ -108,7 +108,7 @@ BLEND_INIT %1, 6, %4
 
         packus%3%2       m0, m1
 
-        mova  [dstq + xq], m0
+        movu  [dstq + xq], m0
         add            xq, mmsize
     jl .loop
 BLEND_END
@@ -148,7 +148,7 @@ BLEND_INIT multiply, 6
         MULTIPLY        m1, m3, m5
 
         packuswb       m0, m1
-        mova  [dstq + xq], m0
+        movu  [dstq + xq], m0
         add            xq, mmsize
     jl .loop
 BLEND_END
@@ -175,7 +175,7 @@ BLEND_INIT screen, 7
         SCREEN          m1, m3, m5, m6
 
         packuswb       m0, m1
-        mova  [dstq + xq], m0
+        movu  [dstq + xq], m0
         add            xq, mmsize
     jl .loop
 BLEND_END
@@ -196,7 +196,7 @@ BLEND_INIT %1, 3, %3
     pxor           m1, m2
     pavg%2         m0, m1
     pxor           m0, m2
-    mova  [dstq + xq], m0
+    movu  [dstq + xq], m0
     add            xq, mmsize
     jl .loop
 BLEND_END
@@ -230,7 +230,7 @@ BLEND_INIT %1, 6, %4
 
         packus%3%2     m0, m1
 
-        mova  [dstq + xq], m0
+        movu  [dstq + xq], m0
         add            xq, mmsize
     jl .loop
 BLEND_END
@@ -251,7 +251,7 @@ BLEND_INIT hardmix, 5
         pxor            m0, m3
         pcmpgtb         m1, m0
         pxor            m1, m2
-        mova   [dstq + xq], m1
+        movu   [dstq + xq], m1
         add             xq, mmsize
     jl .loop
 BLEND_END
@@ -304,7 +304,7 @@ BLEND_INIT %1, 4, %3
         mova            m2, m3
         psubus%2        m2, m1
         paddus%2        m2, m0
-        mova   [dstq + xq], m2
+        movu   [dstq + xq], m2
         add             xq, mmsize
     jl .loop
 BLEND_END
@@ -333,7 +333,7 @@ BLEND_INIT %1, 5, %4
         ABS2            m0, m3, m1, m4
 %endif
         packus%3%2      m0, m3
-        mova   [dstq + xq], m0
+        movu   [dstq + xq], m0
         add             xq, mmsize
     jl .loop
 BLEND_END
@@ -369,7 +369,7 @@ BLEND_INIT %1, 8, %4
         ABS2            m3, m7, m1, m6
 %endif
         packus%3%2      m3, m7
-        mova   [dstq + xq], m3
+        movu   [dstq + xq], m3
         add             xq, mmsize
     jl .loop
 BLEND_END
@@ -406,7 +406,7 @@ BLEND_INIT %1, 8, %4
         psub%3          m0, m4, m3
         psub%3          m1, m4, m7
         packus%3%2      m0, m1
-        mova   [dstq + xq], m0
+        movu   [dstq + xq], m0
         add             xq, mmsize
     jl .loop
 BLEND_END
-- 
2.31.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/4] avfilter/vf_zscale: fix number of threads

[Marton Balint]

Make sure it is between [1, MAX_THERADS] and also take into account the outlink
size in order not to request zero height output from zscale.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/vf_zscale.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index bb457423b3..bab87b0c94 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -799,7 +799,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
         link->dst->inputs[0]->w      = in->width;
         link->dst->inputs[0]->h      = in->height;
 
-        s->nb_threads = FFMIN(ff_filter_get_nb_threads(ctx), link->h / 8);
+        s->nb_threads = av_clip(FFMIN(ff_filter_get_nb_threads(ctx), FFMIN(link->h, outlink->h) / 8), 1, MAX_THREADS);
         s->in_colorspace = in->colorspace;
         s->in_trc = in->color_trc;
         s->in_primaries = in->color_primaries;
-- 
2.31.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/4] avfilter/vf_zscale: fix number of threads

[Paul B Mahol]

lgtm
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/4] avfilter/vf_zscale: fix number of threads

[Victoria Zhislina]

Thanks for the fix, lgtm as well.

On Tue, Mar 15, 2022 at 12:56 AM Paul B Mahol <onemda@gmail.com> wrote:

> lgtm
> _______________________________________________
> 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".
>
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/4] avfilter/vf_zscale: fix number of threads

[Marton Balint]

On Wed, 16 Mar 2022, Victoria Zhislina wrote:

> Thanks for the fix, lgtm as well.

Thanks, applied the zscale patches in the series.

Regards,
Marton

>
> On Tue, Mar 15, 2022 at 12:56 AM Paul B Mahol <onemda@gmail.com> wrote:
>
>> lgtm
>> _______________________________________________
>> 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".
>>
> _______________________________________________
> 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".
>
_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/4] avfilter/vf_zscale: properly check return value of slice threads

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/vf_zscale.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index bab87b0c94..ceefc95224 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -117,6 +117,7 @@ typedef struct ZScaleContext {
 
     void *tmp[MAX_THREADS]; //separate for each thread;
     int nb_threads;
+    int jobs_ret[MAX_THREADS];
 
     zimg_image_format src_format, dst_format;
     zimg_image_format alpha_src_format, alpha_dst_format;
@@ -858,12 +859,14 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
         td.desc = desc;
         td.odesc = odesc;
 
-        ret = ff_filter_execute(ctx, filter_slice, &td, NULL, s->nb_threads);
-        if (ret < 0 || !s->graph[0]) {
+        memset(s->jobs_ret, 0, s->nb_threads * sizeof(*s->jobs_ret));
+        ret = ff_filter_execute(ctx, filter_slice, &td, s->jobs_ret, s->nb_threads);
+        for (int i = 0; ret >= 0 && i < s->nb_threads; i++)
+            if (s->jobs_ret[i] < 0)
+                ret = s->jobs_ret[i];
+        if (ret < 0) {
             av_frame_free(&in);
             av_frame_free(&out);
-            if (ret >= 0)
-                ret = AVERROR(EINVAL);
             return ret;
         }
 
-- 
2.31.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/4] avfilter/vf_zscale: properly check return value of slice threads

[Paul B Mahol]

lgtm
_______________________________________________
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".

[FFmpeg-devel] [PATCH 4/4] avfilter/vf_zscale: realign output buffer if needed

[Marton Balint]

Output buffer alignment might be different to ZIMG_ALIGNMENT or it may not be
aligned at all if a downstream filter (e.g. vf_pad) intentionally misaligns it.

Or maybe we should unconditionally always allocate output with
av_frame_get_buffer() instead of ff_get_video_buffer()?

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/vf_zscale.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index ceefc95224..2061e38bcc 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -632,7 +632,7 @@ static int graphs_build(AVFrame *in, AVFrame *out, const AVPixFmtDescriptor *des
     return 0;
 }
 
-static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame **frame)
+static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame **frame, int needs_copy)
 {
     AVFrame *aligned = NULL;
     int ret = 0, plane, planes;
@@ -654,10 +654,10 @@ static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame **frame)
             if ((ret = av_frame_get_buffer(aligned, ZIMG_ALIGNMENT)) < 0)
                 goto fail;
 
-            if ((ret = av_frame_copy(aligned, *frame)) < 0)
+            if (needs_copy && (ret = av_frame_copy(aligned, *frame)) < 0)
                 goto fail;
 
-            if ((ret = av_frame_copy_props(aligned, *frame)) < 0)
+            if (needs_copy && (ret = av_frame_copy_props(aligned, *frame)) < 0)
                 goto fail;
 
             av_frame_free(frame);
@@ -786,9 +786,12 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
             goto fail;
         }
 
+        if ((ret = realign_frame(odesc, &out, 0)) < 0)
+            goto fail;
+
         av_frame_copy_props(out, in);
 
-        if ((ret = realign_frame(desc, &in)) < 0)
+        if ((ret = realign_frame(desc, &in, 1)) < 0)
             goto fail;
 
         snprintf(buf, sizeof(buf)-1, "%d", outlink->w);
-- 
2.31.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/4] avfilter/vf_zscale: realign output buffer if needed

[Paul B Mahol]

lgtm
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/4] avfilter/vf_zscale: realign output buffer if needed

[James Almer]

On 3/14/2022 6:06 PM, Marton Balint wrote:
> Output buffer alignment might be different to ZIMG_ALIGNMENT or it may not be
> aligned at all if a downstream filter (e.g. vf_pad) intentionally misaligns it.

av_frame_get_buffer() align parameter is used to align linesizes, not 
buffers. av_malloc() has a hardcoded alignment defined at compile time 
based on configure settings.

If what you need is aligned data pointers, you should either use the 
trick i used in the libdav1d wrapper, or change av_malloc() in order to 
use av_cpu_max_align().

> 
> Or maybe we should unconditionally always allocate output with
> av_frame_get_buffer() instead of ff_get_video_buffer()?
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   libavfilter/vf_zscale.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
> index ceefc95224..2061e38bcc 100644
> --- a/libavfilter/vf_zscale.c
> +++ b/libavfilter/vf_zscale.c
> @@ -632,7 +632,7 @@ static int graphs_build(AVFrame *in, AVFrame *out, const AVPixFmtDescriptor *des
>       return 0;
>   }
>   
> -static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame **frame)
> +static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame **frame, int needs_copy)
>   {
>       AVFrame *aligned = NULL;
>       int ret = 0, plane, planes;
> @@ -654,10 +654,10 @@ static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame **frame)
>               if ((ret = av_frame_get_buffer(aligned, ZIMG_ALIGNMENT)) < 0)
>                   goto fail;
>   
> -            if ((ret = av_frame_copy(aligned, *frame)) < 0)
> +            if (needs_copy && (ret = av_frame_copy(aligned, *frame)) < 0)
>                   goto fail;
>   
> -            if ((ret = av_frame_copy_props(aligned, *frame)) < 0)
> +            if (needs_copy && (ret = av_frame_copy_props(aligned, *frame)) < 0)
>                   goto fail;
>   
>               av_frame_free(frame);
> @@ -786,9 +786,12 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>               goto fail;
>           }
>   
> +        if ((ret = realign_frame(odesc, &out, 0)) < 0)
> +            goto fail;
> +
>           av_frame_copy_props(out, in);
>   
> -        if ((ret = realign_frame(desc, &in)) < 0)
> +        if ((ret = realign_frame(desc, &in, 1)) < 0)
>               goto fail;
>   
>           snprintf(buf, sizeof(buf)-1, "%d", outlink->w);
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/4] avfilter/vf_zscale: realign output buffer if needed

[James Almer]

On 3/16/2022 5:52 PM, James Almer wrote:
> On 3/14/2022 6:06 PM, Marton Balint wrote:
>> Output buffer alignment might be different to ZIMG_ALIGNMENT or it may 
>> not be
>> aligned at all if a downstream filter (e.g. vf_pad) intentionally 
>> misaligns it.
> 
> av_frame_get_buffer() align parameter is used to align linesizes, not 
> buffers. av_malloc() has a hardcoded alignment defined at compile time 
> based on configure settings.
> 
> If what you need is aligned data pointers, you should either use the 
> trick i used in the libdav1d wrapper, or change av_malloc() in order to 
> use av_cpu_max_align().
> 
>>
>> Or maybe we should unconditionally always allocate output with
>> av_frame_get_buffer() instead of ff_get_video_buffer()?

Also, ff_get_video_buffer() can be made to accept an align parameter, 
much like av_frame_get_buffer().
The latter (and av_frame_make_writable) should not be used in lavfi 
since you're losing the benefits from the AVFilterLink buffer pool.

>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>   libavfilter/vf_zscale.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
>> index ceefc95224..2061e38bcc 100644
>> --- a/libavfilter/vf_zscale.c
>> +++ b/libavfilter/vf_zscale.c
>> @@ -632,7 +632,7 @@ static int graphs_build(AVFrame *in, AVFrame *out, 
>> const AVPixFmtDescriptor *des
>>       return 0;
>>   }
>> -static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame 
>> **frame)
>> +static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame 
>> **frame, int needs_copy)
>>   {
>>       AVFrame *aligned = NULL;
>>       int ret = 0, plane, planes;
>> @@ -654,10 +654,10 @@ static int realign_frame(const 
>> AVPixFmtDescriptor *desc, AVFrame **frame)
>>               if ((ret = av_frame_get_buffer(aligned, ZIMG_ALIGNMENT)) 
>> < 0)
>>                   goto fail;
>> -            if ((ret = av_frame_copy(aligned, *frame)) < 0)
>> +            if (needs_copy && (ret = av_frame_copy(aligned, *frame)) 
>> < 0)
>>                   goto fail;
>> -            if ((ret = av_frame_copy_props(aligned, *frame)) < 0)
>> +            if (needs_copy && (ret = av_frame_copy_props(aligned, 
>> *frame)) < 0)
>>                   goto fail;
>>               av_frame_free(frame);
>> @@ -786,9 +786,12 @@ static int filter_frame(AVFilterLink *link, 
>> AVFrame *in)
>>               goto fail;
>>           }
>> +        if ((ret = realign_frame(odesc, &out, 0)) < 0)
>> +            goto fail;
>> +
>>           av_frame_copy_props(out, in);
>> -        if ((ret = realign_frame(desc, &in)) < 0)
>> +        if ((ret = realign_frame(desc, &in, 1)) < 0)
>>               goto fail;
>>           snprintf(buf, sizeof(buf)-1, "%d", outlink->w);
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/4] avfilter/vf_zscale: realign output buffer if needed

[Marton Balint]

On Wed, 16 Mar 2022, James Almer wrote:

> On 3/14/2022 6:06 PM, Marton Balint wrote:
>>  Output buffer alignment might be different to ZIMG_ALIGNMENT or it may not
>>  be
>>  aligned at all if a downstream filter (e.g. vf_pad) intentionally
>>  misaligns it.
>
> av_frame_get_buffer() align parameter is used to align linesizes, not 
> buffers. av_malloc() has a hardcoded alignment defined at compile time based 
> on configure settings.

Good point.

>
> If what you need is aligned data pointers, you should either use the trick i 
> used in the libdav1d wrapper, or change av_malloc() in order to use 
> av_cpu_max_align().

Actually av_cpu_max_align() also depends on compile time, so your trick is 
the only way to ensure ZIMG alignment as far as I see...

Regards,
Marton
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/4] avfilter/x86/vf_blend: use unaligned movs for output

[Paul B Mahol]

lgtm
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/4] avfilter/x86/vf_blend: use unaligned movs for output

[Marton Balint]

On Fri, 18 Mar 2022, Paul B Mahol wrote:

> lgtm

Thanks, applied.

Regards,
Marton
_______________________________________________
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".