diff mbox series

[FFmpeg-devel] avfilter/af_adelay: make per channel delay argument an int64_t

Message ID 20210423134800.724-1-jamrial@gmail.com
State Accepted
Commit bc2726969400e1e57d25d1042f860eb2cbdf7465
Headers show
Series [FFmpeg-devel] avfilter/af_adelay: make per channel delay argument an int64_t
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished

Commit Message

James Almer April 23, 2021, 1:48 p.m. UTC
Should fix ticket #9196

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavfilter/af_adelay.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Andreas Rheinhardt April 23, 2021, 2:18 p.m. UTC | #1
James Almer:
> Should fix ticket #9196
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavfilter/af_adelay.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c
> index 6ac81c2a3e..81ff7947f5 100644
> --- a/libavfilter/af_adelay.c
> +++ b/libavfilter/af_adelay.c
> @@ -28,9 +28,9 @@
>  #include "internal.h"
>  
>  typedef struct ChanDelay {
> -    int delay;
> -    unsigned delay_index;
> -    unsigned index;
> +    int64_t delay;
> +    size_t delay_index;
> +    size_t index;
>      uint8_t *samples;
>  } ChanDelay;
>  
> @@ -152,7 +152,7 @@ static int config_input(AVFilterLink *inlink)
>  
>          p = NULL;
>  
> -        ret = av_sscanf(arg, "%d%c", &d->delay, &type);
> +        ret = av_sscanf(arg, "%"SCNd64"%c", &d->delay, &type);
>          if (ret != 2 || type != 'S') {
>              div = type == 's' ? 1.0 : 1000.0;
>              if (av_sscanf(arg, "%f", &delay) != 1) {
> @@ -194,6 +194,11 @@ static int config_input(AVFilterLink *inlink)
>          if (!d->delay)
>              continue;
>  
> +        if (d->delay > SIZE_MAX) {

Does this give a new compiler warning on 64bit systems (where this check
is tautologically false)?

> +            av_log(ctx, AV_LOG_ERROR, "Requested delay is too big.\n");
> +            return AVERROR(EINVAL);
> +        }
> +
>          d->samples = av_malloc_array(d->delay, s->block_align);
>          if (!d->samples)
>              return AVERROR(ENOMEM);
>
James Almer April 23, 2021, 2:33 p.m. UTC | #2
On 4/23/2021 11:18 AM, Andreas Rheinhardt wrote:
> James Almer:
>> Should fix ticket #9196
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavfilter/af_adelay.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c
>> index 6ac81c2a3e..81ff7947f5 100644
>> --- a/libavfilter/af_adelay.c
>> +++ b/libavfilter/af_adelay.c
>> @@ -28,9 +28,9 @@
>>   #include "internal.h"
>>   
>>   typedef struct ChanDelay {
>> -    int delay;
>> -    unsigned delay_index;
>> -    unsigned index;
>> +    int64_t delay;
>> +    size_t delay_index;
>> +    size_t index;
>>       uint8_t *samples;
>>   } ChanDelay;
>>   
>> @@ -152,7 +152,7 @@ static int config_input(AVFilterLink *inlink)
>>   
>>           p = NULL;
>>   
>> -        ret = av_sscanf(arg, "%d%c", &d->delay, &type);
>> +        ret = av_sscanf(arg, "%"SCNd64"%c", &d->delay, &type);
>>           if (ret != 2 || type != 'S') {
>>               div = type == 's' ? 1.0 : 1000.0;
>>               if (av_sscanf(arg, "%f", &delay) != 1) {
>> @@ -194,6 +194,11 @@ static int config_input(AVFilterLink *inlink)
>>           if (!d->delay)
>>               continue;
>>   
>> +        if (d->delay > SIZE_MAX) {
> 
> Does this give a new compiler warning on 64bit systems (where this check
> is tautologically false)?

Not here with GCC 10.2.0 mingw-w64, at least.
With -Wextra it prints a -Wsign-compare warning, but it gets buried in a 
dozen other similar warnings.

Do you prefer if i make the check "d->delay > SIZE_MAX / s->block_align" 
instead?

> 
>> +            av_log(ctx, AV_LOG_ERROR, "Requested delay is too big.\n");
>> +            return AVERROR(EINVAL);
>> +        }
>> +
>>           d->samples = av_malloc_array(d->delay, s->block_align);
>>           if (!d->samples)
>>               return AVERROR(ENOMEM);
>>
> 
> _______________________________________________
> 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".
>
Andreas Rheinhardt April 23, 2021, 2:39 p.m. UTC | #3
James Almer:
> On 4/23/2021 11:18 AM, Andreas Rheinhardt wrote:
>> James Almer:
>>> Should fix ticket #9196
>>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>>   libavfilter/af_adelay.c | 13 +++++++++----
>>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c
>>> index 6ac81c2a3e..81ff7947f5 100644
>>> --- a/libavfilter/af_adelay.c
>>> +++ b/libavfilter/af_adelay.c
>>> @@ -28,9 +28,9 @@
>>>   #include "internal.h"
>>>     typedef struct ChanDelay {
>>> -    int delay;
>>> -    unsigned delay_index;
>>> -    unsigned index;
>>> +    int64_t delay;
>>> +    size_t delay_index;
>>> +    size_t index;
>>>       uint8_t *samples;
>>>   } ChanDelay;
>>>   @@ -152,7 +152,7 @@ static int config_input(AVFilterLink *inlink)
>>>             p = NULL;
>>>   -        ret = av_sscanf(arg, "%d%c", &d->delay, &type);
>>> +        ret = av_sscanf(arg, "%"SCNd64"%c", &d->delay, &type);
>>>           if (ret != 2 || type != 'S') {
>>>               div = type == 's' ? 1.0 : 1000.0;
>>>               if (av_sscanf(arg, "%f", &delay) != 1) {
>>> @@ -194,6 +194,11 @@ static int config_input(AVFilterLink *inlink)
>>>           if (!d->delay)
>>>               continue;
>>>   +        if (d->delay > SIZE_MAX) {
>>
>> Does this give a new compiler warning on 64bit systems (where this check
>> is tautologically false)?
> 
> Not here with GCC 10.2.0 mingw-w64, at least.
> With -Wextra it prints a -Wsign-compare warning, but it gets buried in a
> dozen other similar warnings.
> 
> Do you prefer if i make the check "d->delay > SIZE_MAX / s->block_align"
> instead?
> 

Of course not. Then the compiler could not optimize the check (and the
string) away.

>>
>>> +            av_log(ctx, AV_LOG_ERROR, "Requested delay is too big.\n");
>>> +            return AVERROR(EINVAL);
>>> +        }
>>> +
>>>           d->samples = av_malloc_array(d->delay, s->block_align);
>>>           if (!d->samples)
>>>               return AVERROR(ENOMEM);
>>>
Paul B Mahol April 25, 2021, 9:06 a.m. UTC | #4
Please apply.

On Fri, Apr 23, 2021 at 4:39 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> James Almer:
> > On 4/23/2021 11:18 AM, Andreas Rheinhardt wrote:
> >> James Almer:
> >>> Should fix ticket #9196
> >>>
> >>> Signed-off-by: James Almer <jamrial@gmail.com>
> >>> ---
> >>>   libavfilter/af_adelay.c | 13 +++++++++----
> >>>   1 file changed, 9 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c
> >>> index 6ac81c2a3e..81ff7947f5 100644
> >>> --- a/libavfilter/af_adelay.c
> >>> +++ b/libavfilter/af_adelay.c
> >>> @@ -28,9 +28,9 @@
> >>>   #include "internal.h"
> >>>     typedef struct ChanDelay {
> >>> -    int delay;
> >>> -    unsigned delay_index;
> >>> -    unsigned index;
> >>> +    int64_t delay;
> >>> +    size_t delay_index;
> >>> +    size_t index;
> >>>       uint8_t *samples;
> >>>   } ChanDelay;
> >>>   @@ -152,7 +152,7 @@ static int config_input(AVFilterLink *inlink)
> >>>             p = NULL;
> >>>   -        ret = av_sscanf(arg, "%d%c", &d->delay, &type);
> >>> +        ret = av_sscanf(arg, "%"SCNd64"%c", &d->delay, &type);
> >>>           if (ret != 2 || type != 'S') {
> >>>               div = type == 's' ? 1.0 : 1000.0;
> >>>               if (av_sscanf(arg, "%f", &delay) != 1) {
> >>> @@ -194,6 +194,11 @@ static int config_input(AVFilterLink *inlink)
> >>>           if (!d->delay)
> >>>               continue;
> >>>   +        if (d->delay > SIZE_MAX) {
> >>
> >> Does this give a new compiler warning on 64bit systems (where this check
> >> is tautologically false)?
> >
> > Not here with GCC 10.2.0 mingw-w64, at least.
> > With -Wextra it prints a -Wsign-compare warning, but it gets buried in a
> > dozen other similar warnings.
> >
> > Do you prefer if i make the check "d->delay > SIZE_MAX / s->block_align"
> > instead?
> >
>
> Of course not. Then the compiler could not optimize the check (and the
> string) away.
>
> >>
> >>> +            av_log(ctx, AV_LOG_ERROR, "Requested delay is too
> big.\n");
> >>> +            return AVERROR(EINVAL);
> >>> +        }
> >>> +
> >>>           d->samples = av_malloc_array(d->delay, s->block_align);
> >>>           if (!d->samples)
> >>>               return AVERROR(ENOMEM);
> >>>
> _______________________________________________
> 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".
>
James Almer April 25, 2021, 10:14 p.m. UTC | #5
On 4/25/2021 6:06 AM, Paul B Mahol wrote:
> Please apply.

Applied it earlier today, but forgot to reply, sorry.
diff mbox series

Patch

diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c
index 6ac81c2a3e..81ff7947f5 100644
--- a/libavfilter/af_adelay.c
+++ b/libavfilter/af_adelay.c
@@ -28,9 +28,9 @@ 
 #include "internal.h"
 
 typedef struct ChanDelay {
-    int delay;
-    unsigned delay_index;
-    unsigned index;
+    int64_t delay;
+    size_t delay_index;
+    size_t index;
     uint8_t *samples;
 } ChanDelay;
 
@@ -152,7 +152,7 @@  static int config_input(AVFilterLink *inlink)
 
         p = NULL;
 
-        ret = av_sscanf(arg, "%d%c", &d->delay, &type);
+        ret = av_sscanf(arg, "%"SCNd64"%c", &d->delay, &type);
         if (ret != 2 || type != 'S') {
             div = type == 's' ? 1.0 : 1000.0;
             if (av_sscanf(arg, "%f", &delay) != 1) {
@@ -194,6 +194,11 @@  static int config_input(AVFilterLink *inlink)
         if (!d->delay)
             continue;
 
+        if (d->delay > SIZE_MAX) {
+            av_log(ctx, AV_LOG_ERROR, "Requested delay is too big.\n");
+            return AVERROR(EINVAL);
+        }
+
         d->samples = av_malloc_array(d->delay, s->block_align);
         if (!d->samples)
             return AVERROR(ENOMEM);