Skip to content
Permalink
Browse files

lavc/aarch64/simple_idct: fix idct_col4_top coefficient

Fixes regression introduced by 5d0b8b1.
  • Loading branch information
mbouron committed Jun 13, 2017
1 parent 4cc2a35 commit 8aa60606fb64b8280627935b0df55d4d2aeca5d1
Showing with 1 addition and 1 deletion.
  1. +1 −1 libavcodec/aarch64/simple_idct_neon.S
@@ -74,7 +74,7 @@ endconst
.endm

.macro idct_col4_top y1, y2, y3, y4, i, l
smull\i v7.4S, \y3\l, z1
smull\i v7.4S, \y3\l, z2
smull\i v16.4S, \y3\l, z6
smull\i v17.4S, \y2\l, z1
add v19.4S, v23.4S, v7.4S

14 comments on commit 8aa6060

@Memphiz

This comment has been minimized.

Copy link
Contributor

@Memphiz Memphiz replied Jun 13, 2017

Do you have any more fixes to come? I still get this errors when compiling with xcode 6.1.1 for ios arm64 and gas-preprocessor:
https://pastebin.com/Cmzbs2Ag

@mbouron

This comment has been minimized.

Copy link
Contributor Author

@mbouron mbouron replied Jun 14, 2017

I'll work on it in the upcoming days.

@Memphiz

This comment has been minimized.

Copy link
Contributor

@Memphiz Memphiz replied Jun 14, 2017

I tried it with xcode7.2 now with and without gas-preprocessor (we have nothing newer on our jenkins atm and can't easily get to xcode8 due to using osx 10.10.x atm) - no dice it just can't get past this file. With which xcode version should this be compilable and is usage of gas-preprocessorcurrently needed or not?

@Memphiz

This comment has been minimized.

Copy link
Contributor

@Memphiz Memphiz replied Jun 14, 2017

Sorry you posted while i was typing - i'll wait then ;)

@mbouron

This comment has been minimized.

Copy link
Contributor Author

@mbouron mbouron replied Jun 14, 2017

The following patch fixes the build with Xcode 7.2 : https://patchwork.ffmpeg.org/patch/3975/
Can you confirm it works for you ?

@Memphiz

This comment has been minimized.

Copy link
Contributor

@Memphiz Memphiz replied Jun 14, 2017

Yes this fixes simple_idct_neon.S ... next thing that doesn't compile is this:

https://pastebin.com/G4VeDe8z

Replacing the "-" with a "," doesn't fix it. Having no idea from arm asm i suppose the "-" is a short cut for a register range and the "," is not working (it complains that registers must be sequential)

@Memphiz

This comment has been minimized.

Copy link
Contributor

@Memphiz Memphiz replied Jun 14, 2017

Ok i got the next 2 problems solved i guess - have a look at this patch please (as i am only guessing asm here ;) ):

https://pastebin.com/Pde1YqMR

Sadly i am out of time now ... the next issue is this:

https://pastebin.com/KXwbkZCP

@Memphiz

This comment has been minimized.

Copy link
Contributor

@Memphiz Memphiz replied Jun 14, 2017

as a last comment - maybe some of those need to be patched via gas-preprocessor instead - not sure how generic those ios adapted asm codes are (for other compilers i mean).

@Memphiz

This comment has been minimized.

Copy link
Contributor

@Memphiz Memphiz replied Jun 17, 2017

@mbouron in addition to the patch you pasted/committed - this is needed for me to get the compilation finally done.

https://pastebin.com/SwwPuSaK

I have no idea if the semantics are still right. Please check the patch. And in case it is correct it would be great if you could take care of adding it into ffmpeg. (i know you don't use github and i am really not technically able to use mailing lists).

@mbouron

This comment has been minimized.

Copy link
Contributor Author

@mbouron mbouron replied Jun 18, 2017

I will test your patch on monday (I don't have an Apple machine at hand until then) and submit it to the mailing if it's correct. If you can format the patch with git format-patch that would be great.

@Memphiz

This comment has been minimized.

Copy link
Contributor

@Memphiz Memphiz replied Jun 18, 2017

top 4 commits here:

https://github.com/Memphiz/FFmpeg/commits/ios64_xcode7

btw - this also solves compilation for xcode 6.1.1 :)

@Memphiz

This comment has been minimized.

Copy link
Contributor

@Memphiz Memphiz replied Jun 24, 2017

Did you get any chance to give it a shot yet? Not into making any pressure here - its just i try to fit the solution of this into my own spare time windows from time to time ;)

@mbouron

This comment has been minimized.

Copy link
Contributor Author

@mbouron mbouron replied Jun 24, 2017

Sorry for the long reply, I did contact the maintainer of those files and he submitted and pushed your patches to both libav and ffmpeg:
https://ffmpeg.org/pipermail/ffmpeg-devel/2017-June/212675.html

@Memphiz

This comment has been minimized.

Copy link
Contributor

@Memphiz Memphiz replied Jun 24, 2017

thx very much for taking care of that :)

Please sign in to comment.