[FFmpeg-devel] [PATCH] install aes.h, des.h and rc4.h
Sun Feb 8 09:56:57 CET 2009
On Sun, Feb 08, 2009 at 02:11:51AM +0100, Michael Niedermayer wrote:
> And i must admit i feel the same in case of your rant about localtime()
> passing its output to another thread and the original one exiting ...
> people dont do this ...
I also suggested ignoring the issues, nobody seemed particularly
interested, and I also gave other reasons for it (similarly theoretical,
but IMO enough to call it "not a proper solution" when it would be
possible to just pass another pointer and fix all the issues - I have
not yet really found out if the tzset() issue is relevant to use).
> so please first explain me what problem my current code really has,
> how likely this is going to hit us in reality and how bad the consequences
> would be.
It has whatever issues lead to the way it is currently used in
libavformat. Maybe that is actually "none", but giving others an API you
avoid to use yourself seems more than wrong to me, and I am going to
stand by that.
Nevertheless I'd summarize the reasons like this:
> using malloc() (or better av_malloc())
potentially too slow, needs a free(), too.
potentially not available, no alignment (which av_malloc would provide
for free, so no "fiddling" would be needed if we ever need it).
> uint64_t buffer
Well, more or less what my suggested macro was supposed to simplify.
Admittedly that macro is not a beauty, but it seems better to me than
having to do it manually (unless you think it is the alignment code
specifically that makes it much uglier).
I am a bit unhappy that for this use case it will not work with
compilers that do not support variable-size stack arrays.
> IMHO, if we suddenly require more alignment than the original publication
> of the api did. the functions themselfs could do the align and require a
> few bytes more, this is exactly identical to your suggestion but factorizes
> the mess away from the user app and is fully within the current API/ABI.
I would like to point out that I somewhat liked my suggestion because it
provided a function analogous to av_malloc but working on the stack,
which I considered a nice feature (a bit ugly, not quite as much as you
consider it). I assumed it could be used in other cases, too, at least
in the future, thus justifying the API extension.
> ahh and sorry if i sounded rude, it wasnt intended, i just think my
> current code (with better documentation of the alignment) is better than
> what you suggested
Well, I am sure not everything I wrote was particularly civil (sorry,
happens when I try to get a point across) and I didn't feel offended
by anything you wrote.
I do think that your suggested solution doesn't fall into the "clean code"
territory about as much as mine but fails even more in the "ease of use"
category, but I don't claim those to be objective measures.
I do think it is too easy to consider a solution "clean" when you didn't
have to use it in your own code and/or had to sell it to your
co-developers, it has happened to me all too often (and I considered
your solution using av_aes_size a really nice solution until I had to
make asfcrypt.c use it and found the solutions uglier than I imagined).
More information about the ffmpeg-devel