| Message ID | AS8P250MB07447985D1C6D4DACFA426B28F4E2@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | c149d86760aca52c9d289ff1aa835689875e842e |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avfilter/vf_signature: Allocate arrays together | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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".
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
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
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 */
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavfilter/vf_signature.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)