* Re: [FFmpeg-devel] [PATCHv2 1/2] lavc/vorbisdec: use ptrdiff_t to iterate over intptr_t
2022-09-19 15:48 ` James Almer
@ 2022-09-19 16:06 ` Rémi Denis-Courmont
2022-09-19 16:10 ` [FFmpeg-devel] [PATCHv3 1/3] " remi
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Rémi Denis-Courmont @ 2022-09-19 16:06 UTC (permalink / raw)
To: ffmpeg-devel
Le maanantaina 19. syyskuuta 2022, 18.48.11 EEST James Almer a écrit :
> On 9/19/2022 12:35 PM, remi@remlab.net wrote:
> > From: Rémi Denis-Courmont <remi@remlab.net>
> >
> > While this probably never overflows, we are better safe than sorry.
> >
> > The callback prototype should probably also use ptrdiff_t or size_t,
> > but I diggress (this would affect the DSP callback prototype).
>
> If you mean using ptrdiff_t instead of intptr_t, it should be pretty
> straightforward. Effectively only a prototype change and no need to
> touch the asm implementations. It would also put it in line with the
> rest of the codebase.
Unfortunately, they are defined in different headers, <stddef.h> vs <stdint.h>
so it is not quite that straightforward.
--
雷米‧德尼-库尔蒙
http://www.remlab.net/
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread* [FFmpeg-devel] [PATCHv3 1/3] lavc/vorbisdec: use ptrdiff_t to iterate over intptr_t
2022-09-19 15:48 ` James Almer
2022-09-19 16:06 ` Rémi Denis-Courmont
@ 2022-09-19 16:10 ` remi
2022-09-19 16:10 ` [FFmpeg-devel] [PATCHv3 2/3] lavc/vorbisdsp: use ptrdiff_t rather than intptr_t remi
2022-09-19 16:10 ` [FFmpeg-devel] [PATCHv3 3/3] lavc/vorbisdec: use intermediate variables remi
3 siblings, 0 replies; 6+ messages in thread
From: remi @ 2022-09-19 16:10 UTC (permalink / raw)
To: ffmpeg-devel
From: Rémi Denis-Courmont <remi@remlab.net>
While this probably never overflows, we are better safe than sorry.
The callback prototype should probably also use ptrdiff_t or size_t,
but I diggress (this would affect the DSP callback prototype).
---
libavcodec/ppc/vorbisdsp_altivec.c | 4 ++--
libavcodec/vorbisdec.c | 3 +--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/libavcodec/ppc/vorbisdsp_altivec.c b/libavcodec/ppc/vorbisdsp_altivec.c
index 4dabf2dc7d..c298d8cae3 100644
--- a/libavcodec/ppc/vorbisdsp_altivec.c
+++ b/libavcodec/ppc/vorbisdsp_altivec.c
@@ -31,12 +31,12 @@
static void vorbis_inverse_coupling_altivec(float *mag, float *ang,
intptr_t blocksize)
{
- int i;
vector float m, a;
vector bool int t0, t1;
const vector unsigned int v_31 = //XXX
vec_add(vec_add(vec_splat_u32(15),vec_splat_u32(15)),vec_splat_u32(1));
- for (i = 0; i < blocksize; i += 4) {
+
+ for (ptrdiff_t i = 0; i < blocksize; i += 4) {
m = vec_ld(0, mag+i);
a = vec_ld(0, ang+i);
t0 = vec_cmple(m, (vector float)vec_splat_u32(0));
diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
index 38a5367be3..bfc4be6fc6 100644
--- a/libavcodec/vorbisdec.c
+++ b/libavcodec/vorbisdec.c
@@ -1581,8 +1581,7 @@ static inline int vorbis_residue_decode(vorbis_context *vc, vorbis_residue *vr,
void ff_vorbis_inverse_coupling(float *mag, float *ang, intptr_t blocksize)
{
- int i;
- for (i = 0; i < blocksize; i++) {
+ for (ptrdiff_t i = 0; i < blocksize; i++) {
if (mag[i] > 0.0) {
if (ang[i] > 0.0) {
ang[i] = mag[i] - ang[i];
--
2.37.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread* [FFmpeg-devel] [PATCHv3 2/3] lavc/vorbisdsp: use ptrdiff_t rather than intptr_t
2022-09-19 15:48 ` James Almer
2022-09-19 16:06 ` Rémi Denis-Courmont
2022-09-19 16:10 ` [FFmpeg-devel] [PATCHv3 1/3] " remi
@ 2022-09-19 16:10 ` remi
2022-09-19 16:10 ` [FFmpeg-devel] [PATCHv3 3/3] lavc/vorbisdec: use intermediate variables remi
3 siblings, 0 replies; 6+ messages in thread
From: remi @ 2022-09-19 16:10 UTC (permalink / raw)
To: ffmpeg-devel
From: Rémi Denis-Courmont <remi@remlab.net>
... for a difference between pointers.
---
libavcodec/aarch64/vorbisdsp_init.c | 2 +-
libavcodec/arm/vorbisdsp_init_arm.c | 2 +-
libavcodec/ppc/vorbisdsp_altivec.c | 2 +-
libavcodec/vorbis.h | 2 +-
libavcodec/vorbisdec.c | 2 +-
libavcodec/vorbisdsp.h | 4 ++--
libavcodec/x86/vorbisdsp_init.c | 2 +-
7 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/libavcodec/aarch64/vorbisdsp_init.c b/libavcodec/aarch64/vorbisdsp_init.c
index c796f95e61..969343934b 100644
--- a/libavcodec/aarch64/vorbisdsp_init.c
+++ b/libavcodec/aarch64/vorbisdsp_init.c
@@ -22,7 +22,7 @@
#include "libavcodec/vorbisdsp.h"
void ff_vorbis_inverse_coupling_neon(float *mag, float *ang,
- intptr_t blocksize);
+ ptrdiff_t blocksize);
av_cold void ff_vorbisdsp_init_aarch64(VorbisDSPContext *c)
{
diff --git a/libavcodec/arm/vorbisdsp_init_arm.c b/libavcodec/arm/vorbisdsp_init_arm.c
index f4b3d80ef6..acda34f468 100644
--- a/libavcodec/arm/vorbisdsp_init_arm.c
+++ b/libavcodec/arm/vorbisdsp_init_arm.c
@@ -25,7 +25,7 @@
#include "libavcodec/vorbisdsp.h"
void ff_vorbis_inverse_coupling_neon(float *mag, float *ang,
- intptr_t blocksize);
+ ptrdiff_t blocksize);
av_cold void ff_vorbisdsp_init_arm(VorbisDSPContext *c)
{
diff --git a/libavcodec/ppc/vorbisdsp_altivec.c b/libavcodec/ppc/vorbisdsp_altivec.c
index c298d8cae3..8f301705e9 100644
--- a/libavcodec/ppc/vorbisdsp_altivec.c
+++ b/libavcodec/ppc/vorbisdsp_altivec.c
@@ -29,7 +29,7 @@
#if HAVE_ALTIVEC
static void vorbis_inverse_coupling_altivec(float *mag, float *ang,
- intptr_t blocksize)
+ ptrdiff_t blocksize)
{
vector float m, a;
vector bool int t0, t1;
diff --git a/libavcodec/vorbis.h b/libavcodec/vorbis.h
index f80187feee..270855da04 100644
--- a/libavcodec/vorbis.h
+++ b/libavcodec/vorbis.h
@@ -45,7 +45,7 @@ int ff_vorbis_len2vlc(uint8_t *bits, uint32_t *codes, unsigned num);
void ff_vorbis_floor1_render_list(vorbis_floor1_entry * list, int values,
uint16_t *y_list, int *flag,
int multiplier, float * out, int samples);
-void ff_vorbis_inverse_coupling(float *mag, float *ang, intptr_t blocksize);
+void ff_vorbis_inverse_coupling(float *mag, float *ang, ptrdiff_t blocksize);
#define ilog(i) av_log2(2*(i))
diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
index bfc4be6fc6..10d187b82a 100644
--- a/libavcodec/vorbisdec.c
+++ b/libavcodec/vorbisdec.c
@@ -1579,7 +1579,7 @@ static inline int vorbis_residue_decode(vorbis_context *vc, vorbis_residue *vr,
}
}
-void ff_vorbis_inverse_coupling(float *mag, float *ang, intptr_t blocksize)
+void ff_vorbis_inverse_coupling(float *mag, float *ang, ptrdiff_t blocksize)
{
for (ptrdiff_t i = 0; i < blocksize; i++) {
if (mag[i] > 0.0) {
diff --git a/libavcodec/vorbisdsp.h b/libavcodec/vorbisdsp.h
index 7abec4e4b7..1775a92cf2 100644
--- a/libavcodec/vorbisdsp.h
+++ b/libavcodec/vorbisdsp.h
@@ -19,12 +19,12 @@
#ifndef AVCODEC_VORBISDSP_H
#define AVCODEC_VORBISDSP_H
-#include <stdint.h>
+#include <stddef.h>
typedef struct VorbisDSPContext {
/* assume len is a multiple of 4, and arrays are 16-byte aligned */
void (*vorbis_inverse_coupling)(float *mag, float *ang,
- intptr_t blocksize);
+ ptrdiff_t blocksize);
} VorbisDSPContext;
void ff_vorbisdsp_init(VorbisDSPContext *dsp);
diff --git a/libavcodec/x86/vorbisdsp_init.c b/libavcodec/x86/vorbisdsp_init.c
index da9f9e685e..08a3bb2965 100644
--- a/libavcodec/x86/vorbisdsp_init.c
+++ b/libavcodec/x86/vorbisdsp_init.c
@@ -25,7 +25,7 @@
#include "libavcodec/vorbisdsp.h"
void ff_vorbis_inverse_coupling_sse(float *mag, float *ang,
- intptr_t blocksize);
+ ptrdiff_t blocksize);
av_cold void ff_vorbisdsp_init_x86(VorbisDSPContext *dsp)
{
--
2.37.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread* [FFmpeg-devel] [PATCHv3 3/3] lavc/vorbisdec: use intermediate variables
2022-09-19 15:48 ` James Almer
` (2 preceding siblings ...)
2022-09-19 16:10 ` [FFmpeg-devel] [PATCHv3 2/3] lavc/vorbisdsp: use ptrdiff_t rather than intptr_t remi
@ 2022-09-19 16:10 ` remi
3 siblings, 0 replies; 6+ messages in thread
From: remi @ 2022-09-19 16:10 UTC (permalink / raw)
To: ffmpeg-devel
From: Rémi Denis-Courmont <remi@remlab.net>
The compiler cannot infer that the two float vectors do not alias,
causing unnecessary extra loads and serialisation. This patch caches
the two input values in local variables so that compiler can optimise
individual loop iterations.
---
libavcodec/vorbisdec.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
index 10d187b82a..72b8e8e15b 100644
--- a/libavcodec/vorbisdec.c
+++ b/libavcodec/vorbisdec.c
@@ -1581,22 +1581,22 @@ static inline int vorbis_residue_decode(vorbis_context *vc, vorbis_residue *vr,
void ff_vorbis_inverse_coupling(float *mag, float *ang, ptrdiff_t blocksize)
{
- for (ptrdiff_t i = 0; i < blocksize; i++) {
- if (mag[i] > 0.0) {
- if (ang[i] > 0.0) {
- ang[i] = mag[i] - ang[i];
+ for (ptrdiff_t i = 0; i < blocksize; i++) {
+ float angi = ang[i], magi = mag[i];
+
+ if (magi > 0.f) {
+ if (angi > 0.f) {
+ ang[i] = magi - angi;
} else {
- float temp = ang[i];
- ang[i] = mag[i];
- mag[i] += temp;
+ ang[i] = magi;
+ mag[i] = magi + angi;
}
} else {
- if (ang[i] > 0.0) {
- ang[i] += mag[i];
+ if (angi > 0.f) {
+ ang[i] = magi + angi;
} else {
- float temp = ang[i];
- ang[i] = mag[i];
- mag[i] -= temp;
+ ang[i] = magi;
+ mag[i] = magi - angi;
}
}
}
--
2.37.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread