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

[David Conrad]

On Feb 12, 2010, at 2:53 PM, Michael Niedermayer wrote:

> On Fri, Feb 12, 2010 at 07:18:53PM +0000, M?ns Rullg?rd wrote:
>> 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...
> 
> ill revert the offending commit later today unless someone else fixed it
> properly (too many things on my todo list for a proper fix from me)

It's not a proper fix, but I pushed the special casing of vp5/6.

Does something like the attached work to get rid of the format list in get_buffer()?

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ffmpeg-align16.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20100212/723adb8d/attachment.txt>