| Message ID | 20170329110232.22383-3-martin@martin.st |
|---|---|
| State | Committed |
| Headers | show |
On 29/03/2017 13:02, Martin Storsjö wrote:
> Should we settle on using LOCAL_ALIGNED(xx) consistently instead?
I'm all for consistency.
the set looks fine to me btw.
On Wed, Mar 29, 2017 at 02:02:32PM +0300, Martin Storsjö wrote: > Previously, the former form always produced a manually aligned, > padded buffer, while the latter can use DECLARE_ALIGNED, if that > amount of stack alignment is supported. > --- > The LOCAL_ALIGNED(4) case in hevc_mvs.c had to be removed, since we > don't have any LOCAL_ALIGNED_4 macro. > > This avoids having to use the slightly awkward LOCAL_ALIGNED_xx syntax. > Contrary to the patches posted yesterday, this leaves the source > inconsistent, using both syntaxes though. Should we settle on using > LOCAL_ALIGNED(xx) consistently instead? > > That is, this is an alternative to https://patches.libav.org/patch/63160/. I prefer this approach and I think we should settle on LOCAL_ALIGNED(xx). OK Diego
On Wed, 29 Mar 2017, Martin Storsjö wrote: > Previously, the former form always produced a manually aligned, > padded buffer, while the latter can use DECLARE_ALIGNED, if that > amount of stack alignment is supported. > --- > The LOCAL_ALIGNED(4) case in hevc_mvs.c had to be removed, since we > don't have any LOCAL_ALIGNED_4 macro. > > This avoids having to use the slightly awkward LOCAL_ALIGNED_xx syntax. > Contrary to the patches posted yesterday, this leaves the source > inconsistent, using both syntaxes though. Should we settle on using > LOCAL_ALIGNED(xx) consistently instead? > > That is, this is an alternative to https://patches.libav.org/patch/63160/. > --- > libavutil/internal.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) FWIW, I had to amend this patch to include mem.h in internal.h, as it uses DECLARE_ALIGNED which is defined in mem.h. (The same was present in the alternative patch as well, but I didn't notice the issue so far.) // Martin
diff --git a/libavutil/internal.h b/libavutil/internal.h index d96762c75d..75ae680b91 100644 --- a/libavutil/internal.h +++ b/libavutil/internal.h @@ -97,24 +97,24 @@ DECLARE_ALIGNED(a, t, la_##v) s o; \ t (*v) o = la_##v -#define LOCAL_ALIGNED(a, t, v, ...) E1(LOCAL_ALIGNED_A(a, t, v, __VA_ARGS__,,)) +#define LOCAL_ALIGNED(a, t, v, ...) LOCAL_ALIGNED_##a(t, v, __VA_ARGS__) #if HAVE_LOCAL_ALIGNED_8 # define LOCAL_ALIGNED_8(t, v, ...) E1(LOCAL_ALIGNED_D(8, t, v, __VA_ARGS__,,)) #else -# define LOCAL_ALIGNED_8(t, v, ...) LOCAL_ALIGNED(8, t, v, __VA_ARGS__) +# define LOCAL_ALIGNED_8(t, v, ...) E1(LOCAL_ALIGNED_A(8, t, v, __VA_ARGS__,,)) #endif #if HAVE_LOCAL_ALIGNED_16 # define LOCAL_ALIGNED_16(t, v, ...) E1(LOCAL_ALIGNED_D(16, t, v, __VA_ARGS__,,)) #else -# define LOCAL_ALIGNED_16(t, v, ...) LOCAL_ALIGNED(16, t, v, __VA_ARGS__) +# define LOCAL_ALIGNED_16(t, v, ...) E1(LOCAL_ALIGNED_A(16, t, v, __VA_ARGS__,,)) #endif #if HAVE_LOCAL_ALIGNED_32 # define LOCAL_ALIGNED_32(t, v, ...) E1(LOCAL_ALIGNED_D(32, t, v, __VA_ARGS__,,)) #else -# define LOCAL_ALIGNED_32(t, v, ...) LOCAL_ALIGNED(32, t, v, __VA_ARGS__) +# define LOCAL_ALIGNED_32(t, v, ...) E1(LOCAL_ALIGNED_A(32, t, v, __VA_ARGS__,,)) #endif #define FF_ALLOC_OR_GOTO(ctx, p, size, label)\