[FFmpeg-devel] [PATCH 2/3] lavc: add a framework to fix alignment problems.

[Nicolas George]

Le septidi 17 floréal, an CCXXV, Hendrik Leppkes a écrit :
> I don't think this is very practical. We have a bunch of generic DSPs
> and if a codec uses those it would need to be constantly updated if
> the requirements of those DSPs change. Today it may only use SSE and
> be happy with 16-byte alignment, but what if someone adds an AVX2
> implementation, or AVX512?

Well, exactly: pray tell me, with your design what happens when Intel
adds AVX512?

I will tell you what happens: applications will have been written for
32-octets alignment, and as soon as their libavcodec is updated, the
crashes will start.

> Going through all codecs that use some DSP and somehow set their
> alignment value seems rather unfortunate - and on top of that, it'll
> add arch-specific conditions, which leads to a lot of unnecessary
> changes to a bunch of encoders (and filters) over time.

The way I see it, the functions that are used to init the DSP should
return the required alignment. That will require a little change since
they do not take an AVCodecContext argument, but only trivial and
mechanical changes, so even if hundreds of files were touched it is
really not an issue.

Of course, setting a high default value by default is a possible
short-term solution, especially in the branches.

> Right now the general consensus is that frames (and stride) should be
> aligned to the maximum your CPU might require, ie. 32 byte on a AVX2
> CPU, even if this is not documented.

It may have been the consensus for people who work on the vectorized
implementations, but not the general consensus, especially for people
who work on user-facing API.

> I think it would be far better to officially document this. Making
> applications aware of the alignment (which they likely are already
> anyway, otherwise they would crash, even if they had to learn the hard
> way) can make the entire process more efficient, since re-aligning can
> be extremely expensive (ie. high res video) and should be avoided at
> all cost if possible (which it not always is when you crop, for
> example).

Encouraging the applications to be aware of the issue for performances
reasons is a good thing.

Forcing them to do so when it is not convenient, on pain of crash, when
it can be done for them just as easily, not so much.

> There is no particular reason we can't make the framework check the
> alignment and re-align if required to the maximum CPU value (with a
> loud warning, perhaps), but I don't think a per-codec or per-filter
> alignment value is very practical.

If the framework does the realigning, doing it per-codec or per-filter
is almost free. And someone really smart said that "since re-aligning
can be extremely expensive" it "should be avoided at all cost if
possible", doing the realigning only if the codecs needs it is really
the right thing to do.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170506/937f138a/attachment.sig>