[FFmpeg-devel] [PATCH 1/5] avfilter/vf_signature: Allocate arrays together

[FFmpeg-devel] [PATCH 1/5] avfilter/vf_signature: Allocate arrays together

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/vf_signature.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/libavfilter/vf_signature.c b/libavfilter/vf_signature.c
index 4896e8f2c1..eb48bf773d 100644
--- a/libavfilter/vf_signature.c
+++ b/libavfilter/vf_signature.c
@@ -250,14 +250,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
         int64_t* elemsignature;
         uint64_t* sortsignature;
 
-        elemsignature = av_malloc_array(elemcat->elem_count, sizeof(int64_t));
+        elemsignature = av_malloc_array(elemcat->elem_count, 2 * sizeof(int64_t));
         if (!elemsignature)
             return AVERROR(ENOMEM);
-        sortsignature = av_malloc_array(elemcat->elem_count, sizeof(int64_t));
-        if (!sortsignature) {
-            av_freep(&elemsignature);
-            return AVERROR(ENOMEM);
-        }
+        sortsignature = elemsignature + elemcat->elem_count;
 
         for (j = 0; j < elemcat->elem_count; j++) {
             blocksum = 0;
@@ -307,7 +303,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
             f++;
         }
         av_freep(&elemsignature);
-        av_freep(&sortsignature);
     }
 
     /* confidence */
-- 
2.34.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/5] avfilter/signature_lookup: Check for allocation error

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/signature_lookup.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
index ad012ecced..90b1d0eadf 100644
--- a/libavfilter/signature_lookup.c
+++ b/libavfilter/signature_lookup.c
@@ -205,11 +205,15 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
     } hspace_elem;
 
     /* houghspace */
-    hspace_elem** hspace = av_malloc_array(MAX_FRAMERATE, sizeof(hspace_elem *));
+    hspace_elem **hspace = av_mallocz(MAX_FRAMERATE * sizeof(*hspace));
 
+    if (!hspace)
+        return NULL;
     /* initialize houghspace */
     for (i = 0; i < MAX_FRAMERATE; i++) {
         hspace[i] = av_malloc_array(2 * HOUGH_MAX_OFFSET + 1, sizeof(hspace_elem));
+        if (!hspace[i])
+            goto error;
         for (j = 0; j < 2 * HOUGH_MAX_OFFSET + 1; j++) {
             hspace[i][j].score = 0;
             hspace[i][j].dist = 99999;
-- 
2.34.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 3/5] avfilter/signature_lookup: Allocate buffers jointly

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/signature_lookup.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
index 90b1d0eadf..3a42737e1a 100644
--- a/libavfilter/signature_lookup.c
+++ b/libavfilter/signature_lookup.c
@@ -205,15 +205,17 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
     } hspace_elem;
 
     /* houghspace */
-    hspace_elem **hspace = av_mallocz(MAX_FRAMERATE * sizeof(*hspace));
+    hspace_elem **hspace = av_malloc(MAX_FRAMERATE * sizeof(*hspace));
+    hspace_elem *hspaces;
 
     if (!hspace)
         return NULL;
     /* initialize houghspace */
+    hspaces = av_malloc((2 * HOUGH_MAX_OFFSET + 1) * sizeof(*hspaces) * MAX_FRAMERATE);
+    if (!hspaces)
+        goto error;
     for (i = 0; i < MAX_FRAMERATE; i++) {
-        hspace[i] = av_malloc_array(2 * HOUGH_MAX_OFFSET + 1, sizeof(hspace_elem));
-        if (!hspace[i])
-            goto error;
+        hspace[i] = hspaces + i * (2 * HOUGH_MAX_OFFSET + 1);
         for (j = 0; j < 2 * HOUGH_MAX_OFFSET + 1; j++) {
             hspace[i][j].score = 0;
             hspace[i][j].dist = 99999;
@@ -325,10 +327,8 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
         }
     }
     error:
-    for (i = 0; i < MAX_FRAMERATE; i++) {
-        av_freep(&hspace[i]);
-    }
     av_freep(&hspace);
+    av_free(hspaces);
     return cands;
 }
 
-- 
2.34.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 4/5] avfilter/signature_lookup: Remove useless error logs

[Andreas Rheinhardt]

These logs use the wrong loglevel and are uninformative;
and it is of course highly unlikely that a buffer of 56B
can't be allocated.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/signature_lookup.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
index 3a42737e1a..ff0d23c5c7 100644
--- a/libavfilter/signature_lookup.c
+++ b/libavfilter/signature_lookup.c
@@ -297,13 +297,9 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
                 if (hmax < hspace[i][j].score) {
                     if (c == NULL) {
                         c = av_malloc(sizeof(MatchingInfo));
-                        if (!c)
-                            av_log(ctx, AV_LOG_FATAL, "Could not allocate memory");
                         cands = c;
                     } else {
                         c->next = av_malloc(sizeof(MatchingInfo));
-                        if (!c->next)
-                            av_log(ctx, AV_LOG_FATAL, "Could not allocate memory");
                         c = c->next;
 
                     }
-- 
2.34.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 5/5] avfilter/signature_lookup: Avoid branch when adding to linked list

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/signature_lookup.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
index ff0d23c5c7..9c69c02fbf 100644
--- a/libavfilter/signature_lookup.c
+++ b/libavfilter/signature_lookup.c
@@ -187,7 +187,7 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
     size_t i, j, k, l, hmax = 0, score;
     int framerate, offset, l1dist;
     double m;
-    MatchingInfo *cands = NULL, *c = NULL;
+    MatchingInfo cands = { 0 }, *c = &cands;
 
     struct {
         uint8_t size;
@@ -295,16 +295,10 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
         for (i = 0; i < MAX_FRAMERATE; i++) {
             for (j = 0; j < HOUGH_MAX_OFFSET; j++) {
                 if (hmax < hspace[i][j].score) {
-                    if (c == NULL) {
-                        c = av_malloc(sizeof(MatchingInfo));
-                        cands = c;
-                    } else {
-                        c->next = av_malloc(sizeof(MatchingInfo));
-                        c = c->next;
-
-                    }
+                    c->next = av_malloc(sizeof(MatchingInfo));
+                    c = c->next;
                     if (!c) {
-                        sll_free(&cands);
+                        sll_free(&cands.next);
                         goto error;
                     }
                     c->framerateratio = (i+1.0) / 30;
@@ -325,7 +319,7 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
     error:
     av_freep(&hspace);
     av_free(hspaces);
-    return cands;
+    return cands.next;
 }
 
 static int iterate_frame(double frr, FineSignature **a, FineSignature **b, int fcount, int *bcount, int dir)
-- 
2.34.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 1/5] avfilter/vf_signature: Allocate arrays together

[epirat07]

On 14 Feb 2024, at 13:03, Andreas Rheinhardt wrote:

> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavfilter/vf_signature.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/libavfilter/vf_signature.c b/libavfilter/vf_signature.c
> index 4896e8f2c1..eb48bf773d 100644
> --- a/libavfilter/vf_signature.c
> +++ b/libavfilter/vf_signature.c
> @@ -250,14 +250,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
>          int64_t* elemsignature;
>          uint64_t* sortsignature;
>
> -        elemsignature = av_malloc_array(elemcat->elem_count, sizeof(int64_t));
> +        elemsignature = av_malloc_array(elemcat->elem_count, 2 * sizeof(int64_t));
>          if (!elemsignature)
>              return AVERROR(ENOMEM);
> -        sortsignature = av_malloc_array(elemcat->elem_count, sizeof(int64_t));
> -        if (!sortsignature) {
> -            av_freep(&elemsignature);
> -            return AVERROR(ENOMEM);
> -        }
> +        sortsignature = elemsignature + elemcat->elem_count;

Just my 2cents as someone not maintaining this code, so feel free to ignore completely:

IMHO this makes it harder to understand what is going on, does it provide any meaningful
benefit?

At the very least I would suggest to add a comment for the sake of whoever looks a this
code next and tries to grasp what is happening there.

>
>          for (j = 0; j < elemcat->elem_count; j++) {
>              blocksum = 0;
> @@ -307,7 +303,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
>              f++;
>          }
>          av_freep(&elemsignature);
> -        av_freep(&sortsignature);
>      }
>
>      /* confidence */
> -- 
> 2.34.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 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/5] avfilter/vf_signature: Allocate arrays together

[Andreas Rheinhardt]

epirat07@gmail.com:
> On 14 Feb 2024, at 13:03, Andreas Rheinhardt wrote:
> 
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  libavfilter/vf_signature.c | 9 ++-------
>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavfilter/vf_signature.c b/libavfilter/vf_signature.c
>> index 4896e8f2c1..eb48bf773d 100644
>> --- a/libavfilter/vf_signature.c
>> +++ b/libavfilter/vf_signature.c
>> @@ -250,14 +250,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
>>          int64_t* elemsignature;
>>          uint64_t* sortsignature;
>>
>> -        elemsignature = av_malloc_array(elemcat->elem_count, sizeof(int64_t));
>> +        elemsignature = av_malloc_array(elemcat->elem_count, 2 * sizeof(int64_t));
>>          if (!elemsignature)
>>              return AVERROR(ENOMEM);
>> -        sortsignature = av_malloc_array(elemcat->elem_count, sizeof(int64_t));
>> -        if (!sortsignature) {
>> -            av_freep(&elemsignature);
>> -            return AVERROR(ENOMEM);
>> -        }
>> +        sortsignature = elemsignature + elemcat->elem_count;
> 
> Just my 2cents as someone not maintaining this code, so feel free to ignore completely:
> 
> IMHO this makes it harder to understand what is going on, does it provide any meaningful
> benefit?
> 
> At the very least I would suggest to add a comment for the sake of whoever looks a this
> code next and tries to grasp what is happening there.
> 

The benefit is to have less code; e.g. if the second allocation fails,
one needs to have special cleanup code for the first one.

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

Re: [FFmpeg-devel] [PATCH 1/5] avfilter/vf_signature: Allocate arrays together

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavfilter/vf_signature.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/libavfilter/vf_signature.c b/libavfilter/vf_signature.c
> index 4896e8f2c1..eb48bf773d 100644
> --- a/libavfilter/vf_signature.c
> +++ b/libavfilter/vf_signature.c
> @@ -250,14 +250,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
>          int64_t* elemsignature;
>          uint64_t* sortsignature;
>  
> -        elemsignature = av_malloc_array(elemcat->elem_count, sizeof(int64_t));
> +        elemsignature = av_malloc_array(elemcat->elem_count, 2 * sizeof(int64_t));
>          if (!elemsignature)
>              return AVERROR(ENOMEM);
> -        sortsignature = av_malloc_array(elemcat->elem_count, sizeof(int64_t));
> -        if (!sortsignature) {
> -            av_freep(&elemsignature);
> -            return AVERROR(ENOMEM);
> -        }
> +        sortsignature = elemsignature + elemcat->elem_count;
>  
>          for (j = 0; j < elemcat->elem_count; j++) {
>              blocksum = 0;
> @@ -307,7 +303,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
>              f++;
>          }
>          av_freep(&elemsignature);
> -        av_freep(&sortsignature);
>      }
>  
>      /* confidence */

Will apply this patchset tomorrow unless there are objections.

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