diff mbox

[FFmpeg-devel] lavfi/deshake: Check alignment before calling asm init function

Message ID CAB0OVGqzM0caS-sa3fQF3hNuy5xLCzV-NDHbwQYT7sXZ2KRVFQ@mail.gmail.com
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos March 16, 2018, 6:55 p.m. UTC 
2018-03-14 23:50 GMT+01:00, Michael Niedermayer <michael@niedermayer.cc>:
> On Sat, Mar 10, 2018 at 08:50:08PM +0100, Carl Eugen Hoyos wrote:
>> Hi!
>>
>> Attached patch fixes ticket #7078 for me.
>>
>> Please comment, Carl Eugen
>
>>  vf_deshake.c |    8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 9f4517eae88416277aeb5bd5b677159914e9c451
>> 0001-lavfi-deshake-Check-alignment-before-calling-asm-ini.patch
>> From 75ead282c3aa3c214d37e766690e2edd037307c0 Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
>> Date: Sat, 10 Mar 2018 20:46:21 +0100
>> Subject: [PATCH] lavfi/deshake: Check alignment before calling asm init
>>  function.
>>
>> Do this for every frame to make sure dynamic filters do not
>> cause crashes.
>>
>> Fixes ticket #7078.
>> ---
>>  libavfilter/vf_deshake.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c
>> index fb4eb35..75e9990 100644
>> --- a/libavfilter/vf_deshake.c
>> +++ b/libavfilter/vf_deshake.c
>> @@ -342,10 +342,6 @@ static av_cold int init(AVFilterContext *ctx)
>>  {
>>      DeshakeContext *deshake = ctx->priv;
>>
>> -    deshake->sad = av_pixelutils_get_sad_fn(4, 4, 1, deshake); // 16x16,
>> 2nd source unaligned
>> -    if (!deshake->sad)
>> -        return AVERROR(EINVAL);
>> -
>>      deshake->refcount = 20; // XXX: add to options?
>>      deshake->blocksize /= 2;
>>      deshake->blocksize = av_clip(deshake->blocksize, 4, 128);
>> @@ -432,6 +428,10 @@ static int filter_frame(AVFilterLink *link, AVFrame
>> *in)
>>      }
>>      av_frame_copy_props(out, in);
>>
>> +    deshake->sad = av_pixelutils_get_sad_fn(4, 4, !((unsigned
>> long)in->data[0] & 15), deshake); // 16x16, 2nd source unaligned
>> +    if (!deshake->sad)
>> +        return AVERROR(EINVAL);
>
> does this need to check linesize too ?

I think so, new patch attached.

Will push if there are no comments, Carl Eugen

Comments

James Almer March 16, 2018, 7:02 p.m. UTC | #1 
On 3/16/2018 3:55 PM, Carl Eugen Hoyos wrote:
> 2018-03-14 23:50 GMT+01:00, Michael Niedermayer <michael@niedermayer.cc>:
>> On Sat, Mar 10, 2018 at 08:50:08PM +0100, Carl Eugen Hoyos wrote:
>>> Hi!
>>>
>>> Attached patch fixes ticket #7078 for me.
>>>
>>> Please comment, Carl Eugen
>>>  vf_deshake.c |    8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>> 9f4517eae88416277aeb5bd5b677159914e9c451
>>> 0001-lavfi-deshake-Check-alignment-before-calling-asm-ini.patch
>>> From 75ead282c3aa3c214d37e766690e2edd037307c0 Mon Sep 17 00:00:00 2001
>>> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
>>> Date: Sat, 10 Mar 2018 20:46:21 +0100
>>> Subject: [PATCH] lavfi/deshake: Check alignment before calling asm init
>>>  function.
>>>
>>> Do this for every frame to make sure dynamic filters do not
>>> cause crashes.
>>>
>>> Fixes ticket #7078.
>>> ---
>>>  libavfilter/vf_deshake.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c
>>> index fb4eb35..75e9990 100644
>>> --- a/libavfilter/vf_deshake.c
>>> +++ b/libavfilter/vf_deshake.c
>>> @@ -342,10 +342,6 @@ static av_cold int init(AVFilterContext *ctx)
>>>  {
>>>      DeshakeContext *deshake = ctx->priv;
>>>
>>> -    deshake->sad = av_pixelutils_get_sad_fn(4, 4, 1, deshake); // 16x16,
>>> 2nd source unaligned
>>> -    if (!deshake->sad)
>>> -        return AVERROR(EINVAL);
>>> -
>>>      deshake->refcount = 20; // XXX: add to options?
>>>      deshake->blocksize /= 2;
>>>      deshake->blocksize = av_clip(deshake->blocksize, 4, 128);
>>> @@ -432,6 +428,10 @@ static int filter_frame(AVFilterLink *link, AVFrame
>>> *in)
>>>      }
>>>      av_frame_copy_props(out, in);
>>>
>>> +    deshake->sad = av_pixelutils_get_sad_fn(4, 4, !((unsigned
>>> long)in->data[0] & 15), deshake); // 16x16, 2nd source unaligned
>>> +    if (!deshake->sad)
>>> +        return AVERROR(EINVAL);
>> does this need to check linesize too ?
> I think so, new patch attached.
> 
> Will push if there are no comments, Carl Eugen
> 
> 
> 0001-lavfi-deshake-Check-alignment-before-calling-asm-ini.patch
> 
> 
> From 768a2cbcb0536f79fd7590186885c60de039afd5 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> Date: Fri, 16 Mar 2018 19:54:03 +0100
> Subject: [PATCH] lavfi/deshake: Check alignment before calling asm init
>  function.
> 
> Do this for every frame to make sure dynamic filters do not
> cause crashes.
> 
> Fixes ticket #7078.
> ---
>  libavfilter/vf_deshake.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c
> index fb4eb35..aa92cef 100644
> --- a/libavfilter/vf_deshake.c
> +++ b/libavfilter/vf_deshake.c
> @@ -342,10 +342,6 @@ static av_cold int init(AVFilterContext *ctx)
>  {
>      DeshakeContext *deshake = ctx->priv;
>  
> -    deshake->sad = av_pixelutils_get_sad_fn(4, 4, 1, deshake); // 16x16, 2nd source unaligned
> -    if (!deshake->sad)
> -        return AVERROR(EINVAL);
> -
>      deshake->refcount = 20; // XXX: add to options?
>      deshake->blocksize /= 2;
>      deshake->blocksize = av_clip(deshake->blocksize, 4, 128);
> @@ -424,6 +420,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>      const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
>      const int chroma_width  = AV_CEIL_RSHIFT(link->w, desc->log2_chroma_w);
>      const int chroma_height = AV_CEIL_RSHIFT(link->h, desc->log2_chroma_h);
> +    int aligned;
>  
>      out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
>      if (!out) {
> @@ -432,6 +429,11 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>      }
>      av_frame_copy_props(out, in);
>  
> +    aligned = !((unsigned long)in->data[0] & 15 | in->linesize[0] & 15);

Should be intptr_t, not unsigned long.

> +    deshake->sad = av_pixelutils_get_sad_fn(4, 4, aligned, deshake); // 16x16, 2nd source unaligned
> +    if (!deshake->sad)
> +        return AVERROR(EINVAL);
> +
>      if (deshake->cx < 0 || deshake->cy < 0 || deshake->cw < 0 || deshake->ch < 0) {
>          // Find the most likely global motion for the current frame
>          find_motion(deshake, (deshake->ref == NULL) ? in->data[0] : deshake->ref->data[0], in->data[0], link->w, link->h, in->linesize[0], &t);
> -- 1.7.10.4
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Carl Eugen Hoyos March 20, 2018, 12:12 a.m. UTC | #2 
2018-03-16 20:02 GMT+01:00, James Almer <jamrial@gmail.com>:
> On 3/16/2018 3:55 PM, Carl Eugen Hoyos wrote:

>> +    aligned = !((unsigned long)in->data[0] & 15 | in->linesize[0] & 15);
>
> Should be intptr_t, not unsigned long.

Definitely, thank you!

Pushed with that change, Carl Eugen
diff mbox

Patch

From 768a2cbcb0536f79fd7590186885c60de039afd5 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Fri, 16 Mar 2018 19:54:03 +0100
Subject: [PATCH] lavfi/deshake: Check alignment before calling asm init
 function.

Do this for every frame to make sure dynamic filters do not
cause crashes.

Fixes ticket #7078.
---
 libavfilter/vf_deshake.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c
index fb4eb35..aa92cef 100644
--- a/libavfilter/vf_deshake.c
+++ b/libavfilter/vf_deshake.c
@@ -342,10 +342,6 @@  static av_cold int init(AVFilterContext *ctx)
 {
     DeshakeContext *deshake = ctx->priv;
 
-    deshake->sad = av_pixelutils_get_sad_fn(4, 4, 1, deshake); // 16x16, 2nd source unaligned
-    if (!deshake->sad)
-        return AVERROR(EINVAL);
-
     deshake->refcount = 20; // XXX: add to options?
     deshake->blocksize /= 2;
     deshake->blocksize = av_clip(deshake->blocksize, 4, 128);
@@ -424,6 +420,7 @@  static int filter_frame(AVFilterLink *link, AVFrame *in)
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
     const int chroma_width  = AV_CEIL_RSHIFT(link->w, desc->log2_chroma_w);
     const int chroma_height = AV_CEIL_RSHIFT(link->h, desc->log2_chroma_h);
+    int aligned;
 
     out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
     if (!out) {
@@ -432,6 +429,11 @@  static int filter_frame(AVFilterLink *link, AVFrame *in)
     }
     av_frame_copy_props(out, in);
 
+    aligned = !((unsigned long)in->data[0] & 15 | in->linesize[0] & 15);
+    deshake->sad = av_pixelutils_get_sad_fn(4, 4, aligned, deshake); // 16x16, 2nd source unaligned
+    if (!deshake->sad)
+        return AVERROR(EINVAL);
+
     if (deshake->cx < 0 || deshake->cy < 0 || deshake->cw < 0 || deshake->ch < 0) {
         // Find the most likely global motion for the current frame
         find_motion(deshake, (deshake->ref == NULL) ? in->data[0] : deshake->ref->data[0], in->data[0], link->w, link->h, in->linesize[0], &t);
-- 
1.7.10.4