[FFmpeg-devel] MXF D10 regression tests

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Wed, Mar 18, 2009 at 08:33:48PM +0000, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> > On Wed, Mar 18, 2009 at 04:04:49PM +0100, Reimar D?ffinger wrote:
>> >> On Wed, Mar 18, 2009 at 03:40:52PM +0100, Michael Niedermayer wrote:
>> >> > iam not asking for all of it to be taken into account but rather that
>> >> > there is some framework in which such things can be filled in where
>> >> > they are needed.
>> >> > (we also have avcodec_align_dimensions())
>> >> 
>> >> Well, you are the maintainer, I leave it to you to come up with a
>> >> solution. At least it should be clear enough what exactly is the issue
>> >> and can be fixed without access to a PPC machine.
>> >> 
>> >> > currently STRIDE_ALIGN is 8 for x86* and that works out exactly
>> >> > because 420 and the linesize relations gurantee 16 byte alignment
>> >> > for luma.
>> >> 
>> >> I want to make completely clear: there is nothing in the code that
>> >> guarantees any "linesize relations", it is pure luck it works, and the
>> >> only reason is that the width is required to be a multiple of 16 and
>> >> STRIDE_ALIGN << chroma_x_shift divides (more precisely is equal to)
>> >> this on x86.
>> >> 
>> >> > if 444 is ever added it will need some special handling like svq1 does,
>> >> > also other archs than x86 set STRIDE_ALIGN to 16 while they possibly
>> >> > like x86 really just mean 16 for luma ...
>> >> 
>> >> Since (probably) none of that is documented that won't be fixed anytime
>> >> soon, at least not by someone who does not know the asm by heart already.
>> >
>> > there was a single line of doc until r12178
>> > the code surrounding has changed several times prior
>> >
>> > and i simply do not remember which asm or how many if any need this
>> > linesize relation.
>> >
>> > it is possible that some of the code expecting the linesize relation was
>> > fixed in the distant past to use uvlinesize/stride
>> >
>> > what should be done is
>> > * identify and document which code exactly needs this relation (your patch
>> >   is a start for this)
>> > * find what speed effect it has to fix that code
>> > * depending on the possibility to fix the code without speed regressions
>> >   document which buffers require the relation and fix the code used to
>> >   allocate them
>> > * fix STRIDE_ALIGN for each arch (really a job for the maintainers of that
>> >   archs)
>> 
>> Do we have to do all of that before we can fix the encoding on PPC and
>> ARM?
>
> well, we cant fix the code requireing the relation without knowing where it
> is and without knowing the speed effect.
> That leaves
> * changing STRIDE_ALIGN to 8

It has to be 16, or it will crash.

> * changing default_get_buffer() to maintain the stride relation with a big
>   FIXME.

How big a fixme?

-- 
M?ns Rullg?rd
mans at mansr.com