[FFmpeg-devel] [PATCH] Bunch of accumulated patches...

[Michael Niedermayer]

On Mon, Jan 14, 2008 at 09:09:01PM +0100, Sigbj?rn Skj?ret wrote:
> >> [..Please CC me if you want me to reply, otherwise I'll have to break the thread like now..]
> 
> *cough* -^
> 
> > true still, libavutil is not a trashcan, all functions there must be needed,
> > usefull, clean, ...
> 
> Well, while we're on the whole issue of clean, I've mentioned this in
> semi-private before, but it's probably worth mentioning again, the current
> design of exposing global variables that contain the size of opaque structs
> (like av_aes_size etc) so that the *caller* can (actually must) allocate them
> is extremely unclean IMO,

The alternative, that is using malloc() though has real disadvatages not just a few
people who have a philosophical dislike for it.

For the tree case using *malloc means doubling the memory requirement over a
flat array, that is not an option. And i dont even want to know what the
speed loss would be with malloc.


> and there's even a potentially triggered bug because
> of it in tree.c:av_tree_destroy() where it av_free()s the node (which might
> have been allocated by anything, or even on stack as the docs doesn't really
> tell you that it is required to use av_malloc (not to mention that this means
> it already is dependent on mem-funcs, so what's the point?))...

The tree code originally used av_malloc() and av_free() ive changed half of
the code to be independant of it, av_tree_destroy() hasnt been updated yet.

also you currently either
A. use av_malloc for the tree -> you can use av_tree_destroy()
B. use a flat array -> you call a single av_free() and its gone


> 
> A sane approach would be adding APIs for managing these opaque structs, but
> IIRC you were against that since it would make everything depend on the
> mem-funcs .. well, personally I don't think that's so bad (atleast not as bad
> as it is now)...

See above
If you have a suggestion which would be cleaner while retaining simple code,
high speed and low memory requirements, just say it ...


> 
> > that said i did not say it should not be in libavutil, just that the reason
> > you provide is not an argument toward it
> > one would have to demonstrate that av_fast_realloc() is of practical use, 
> > important, well designed, clean, ...
> 
> So you're argumenting towards the removal of this function from avcodec? ;)

i wanted to remove it, yes
but someone posted benchmarks showing its faster than gnu libc realloc() so i
wont remove it


> 
> >> Ok, then I'm open to suggestions, what is the best suited errorcode? EILSEQ
> >> doesn't define the error any better than ENOEXEC...
> > i dunno, but i wont change a bad one to another bad one and while doing so
> > possibly break some applications
> 
> Ok, so would an acceptable approach be to define AVERROR_NOFMT to something
> else if EILSEQ doesn't exist (then ABI is intact on systems avcodec already
> builds on, and will actually build on the rest)?

there are really several seperate things here
1. non posix systems -> work on libos (see the ML) or fix your system
2. error cases which cant be represented well with posix E* -> send
   patches which define custom E* for us, note, this also needs a
   configure check so that they dont interfere with any posix E*

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080114/688ed38a/attachment.pgp>