[FFmpeg-cvslog] r21736 - trunk/libavcodec/x86/dsputil_mmx.c

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Fri, Feb 12, 2010 at 05:19:03PM +0000, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> > On Thu, Feb 11, 2010 at 03:30:46PM +0000, M?ns Rullg?rd wrote:
>> >> Michael Niedermayer <michaelni at gmx.at> writes:
>> >> 
>> >> > On Wed, Feb 10, 2010 at 05:08:07PM -0500, David Conrad wrote:
>> >> >> On Feb 10, 2010, at 4:16 PM, Alex Converse wrote:
>> >> >> 
>> >> >> > On Tue, Feb 9, 2010 at 9:02 PM, conrad <subversion at mplayerhq.hu> wrote:
>> >> >> >> Author: conrad
>> >> >> >> Date: Wed Feb 10 03:02:06 2010
>> >> >> >> New Revision: 21736
>> >> >> >> 
>> >> >> >> Log:
>> >> >> >> Enable SSE2 (put|avg)_pixels_16_sse2
>> >> >> >> 
>> >> >> >> SVQ1 chroma has been special-cased aligned to 16-bytes
>> >> >> >> since at least r15466 Other architectures also assume
>> >> >> >> 16-byte alignment here too but set STRIDE_ALIGN to 16.
>> >> >> >> 
>> >> >> >> Modified:
>> >> >> >>   trunk/libavcodec/x86/dsputil_mmx.c
>> >> >> >> 
>> >> >> > 
>> >> >> > This broke vp6f:
>> >> >> > http://fate.multimedia.cx/index.php?build_record=178551
>> >> >> > http://fate.multimedia.cx/index.php?build_record=178488
>> >> >> 
>> >> >> Looks like vp5 and vp6 use width 16 pixel functions on chroma
>> >> >> as well. I'd really prefer if SSE used 16-byte aligment like
>> >> >> other architectures (first patch). Otherwise VP5/6 need to be
>> >> >> special-cased as well (second patch), and potentially other
>> >> >> codecs that FATE doesn't test, which gets extremely ugly
>> >> >> fast.
>> >> >
>> >> > i prefer the second patch
>> >> > otherwise extensive benchmaring is needed
>> >> 
>> >> Maybe we could make it a codec capability flag or something instead of
>> >> an ever-growing list of special cases.
>> >
>> > yes, but it should go into a non public header, the user app has
>> > no need to mess with it nor should we have to bump the ABI if we
>> > decide to increase STRIDE_ALIGN one day and remove this
>> 
>> An app providing its own get_buffer would need to know, methinks...
>
> hmm
>
> maybe we should add avcodec_align_dimensions2() that provides
> width/height/lumastride/chromastide alignment values

Fine with me, but I'd really like to see this fixed sooner rather than
later.  FATE is in a very sorry state right now.

> It is not what we do, but why we do it that matters.

And when we do it...

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