[FFmpeg-devel] [PATCH 1/4] x86inc: Support arbitrary stack alignments

[Hendrik Leppkes]

On Mon, Aug 3, 2015 at 5:26 PM, James Almer <jamrial at gmail.com> wrote:
> On 03/08/15 9:50 AM, Hendrik Leppkes wrote:
>> On Mon, Aug 3, 2015 at 10:31 AM, Henrik Gramner <henrik at gramner.com> wrote:
>>> On Mon, Aug 3, 2015 at 2:18 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>>>> So, I think the code changes themselves look mostly healthy. Is there a
>>>> behavioural difference before/after this patch? (Like: were there bugs in
>>>> the original code, or does this change behaviour of previous code in a
>>>> significant way?)
>>>
>>> Should only be what's in the commit message; "Previously alignment
>>> would occur either before or after allocating stack space depending on
>>> whether manual alignment was required or not." which I guess you could
>>> classify as a bug (it certainly wasn't a sensible behavior). It's the
>>> reason for why the weird deblock stack allocation for example existed
>>> in the first place.
>>>
>>> So anything relying on the previous alignment behavior of automatic
>>> stack allocation using cglobal would be affected, other than that it
>>> shouldn't make any difference since ffmpeg doesn't use >16-byte stack
>>> alignment.
>>>
>>> I can only compile ffmpeg with --disable-programs when using
>>> msys2/msvc2015 (ffmpeg.c(437): error C2039: '_cnt': is not a member of
>>> '_iobuf'). Not sure if I'm doing something wrong, but if someone is
>>> able to test that better that would be nice.
>>
>> msvc2015 support is unfortunately still broken. I'll send a patch
>> later for this.
>>
>> Otherwise, I tested the patch with msvc 2013 32-bit, and fate passed fine.
>> If there is something else I should specifically test which may not be
>> covered by fate, let me know.
>
> Just to be sure try to convert an 8ch audio stream from float/s24 to floatp/s24p.
> See if it crashes or gives wrong output.
>

Since I wasn't sure in which direction of packed/planar the problem
usually happened, I tried 8ch s32 -> 8ch s32p, 8ch s32 -> 2ch s32p,
8ch s32p -> 8ch s32 and 8ch s32p -> 2ch s32, and all work fine.

- Hendrik