[FFmpeg-devel] [PATCH] Unbreak Altivec PPC optimizations

[Guillaume POIRIER]

Hello,

On Wed, Dec 24, 2008 at 11:46 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Wed, Dec 24, 2008 at 10:48:34AM +0100, Guillaume POIRIER wrote:
>> Hello folks,
>>
>> Now that we have a "all or none" situation regarding the
>> implementation of  h264_idct8_add, h264_idct8_add4 (same for
>> h264_idct_add, h264_idct_add16, h264_idct_add16intra, h264_idct_add8),
>> Altivec-enabled builds lead to corrupt picture.
>>
>> Since it will take time for me (or anyone else!) to implement all of
>> them, I'd like to suggest the attached patch to temporarily improve
>> the situation.
>
> iam fine wth the patch though iam not altivec maintainer

Ok. I'm listed as PPC/OSX so I feel like I could commit this directly.
I just prefer to post before having to roll back changes that don't
fit you or others.


>> I'm like to point out that in the future, care should be taken to
>> _strictly_ avoid adding such dependencies between accelerated
>> routines, because it's a pain to keep up with such changes for non-x86
>> maintainers.
>
> Id like to point out that this was unavoidable and telling me to avoid
> that next time is just not helping if you cannot even suggest how exactly
> the last (aka the current) one should have been avoided.
> (i hope above doesnt sound too nasty ....)

No, it doesn't sound nasty. Why I wrote this is just to underline that
writing optimized routines takes time, so when this effort is killed
useless because of some _avoidable_ code changes in the C code, then
I'm not happy.

If it wasn't avoidable, then all I care is that people explain what
has to be done to fix the situation without having to go through the
time-consuming problem-hunting process.

You did end-up documenting this, so I can't complain.... but you did
so _after_ you changed the code, that _that_ is what worries me.


> I mean, mans said to give maintainers a 24h warning ahead, this can be
> done but would have made no real difference at all, no single maintainer
> dealt with this in a timeframe even close to 24h after it was known ...
> it just would slow work down by 24h for such changes though then they
> arent that common ...

For sure I can't port SIMD routines to Altivec in a 24h time. But in a
24h time, I can ensure that the port is not broken.


> "Avoiding" the issue by simply not introducing the dependancy would
> require to add complexity and overhead to the code that
> could negate the whole speed gain, besides its work that is known to
> be thrown away. if a new 4x4 idct function is introduced that simply
> must use the same permutations as the already used optimized 4x4 idct
> function.
>
> Thus in the end unless you suggest that some kind of optimizations
> should better not be done at all, this really seems unavoidable. But iam
> surely open to suggestions if you have some ...

I don't have any suggestion to make regarding this. I trust your
judgement when you say that there was no better way around this
change.

Next time, please directly document what consequences your changes
have for non-x86 maintainers.


> The truth and real problem IMHO is that large parts of the non x86 code
> is not maintained at all, and the solution for this cant be to stop
> improving code because it might break unmaintained code.

It's true that there are very few people working on Altivec code, but
as they say "if it's not broken, don't fix it". The fact that it
doesn't get constantly changed doesn't mean that nobody uses it, and
doesn't mean that it doesn't work either.


The situation with PPC isn't going to get better anyway since there
are no new mainstream desktop machines available for people to buy.
So let's try to port maintainers easier by communicating a bit more.

:-)

Guillaume
-- 
Only a very small fraction of our DNA does anything; the rest is all
comments and ifdefs.

Bill Cosby  - "Human beings are the only creatures on earth that allow
their children to come back home."