[FFmpeg-devel] [PATCH] avutil/utils: remove unnecessary things

Michael Niedermayer michaelni at gmx.at
Fri Mar 20 18:33:37 CET 2015


On Fri, Mar 20, 2015 at 05:41:43PM +0100, wm4 wrote:
> The checks for the enums all were added in 2011 and 2012. They were
> likely for checking that Libav changes do not change the FFmpeg ABI.
> Given the amount of changes in the last 3 years, and the fact that
> barely any other ABI specifics are tested, keeping these tests makes
> not much sense anymore.

The tests check that ABI is not broken, for example if someone added
a entry in the middle of a enum it would break ABI to our own
previous release with same so-name.
its better if the assert punches the person who puts that in the
middle of a enum straight in the face instead of it being posted
to the mailing list and someone needing to ask for it to be fixed
and reposted with it moved to the end

Testing for this in a cleaner way is very welcome of course


> 
> The assertion checking for "((size_t)-1) > 0" is useless. The condition

probably ok


[...]

> The av_sat_dadd32() test was added in 2012 to check for broken ARM
> binutils. Even if anyone argues that this is still relevant, it should
> be done in configure instead. (And always should have.) The binutils
> bug report (as cited in the original ffmpeg-devel thread discussing
> this patch) was marked as fixed in 2009.

If this test can be done in configure and is reliable, iam in favor
of doing it in configure as well


> 
> The llrint() was added in 2013 for NetBSD. I don't see what printing a
> message changes. (Apparently nothing; from what I can see NetBSD didn't
> touch its llrint implementation since 2004.) If it's critical, just make
> configure refuse to run if NetBSD is detected, or print a compile-time
> warning, or do not use llrint.

IMO its better to test for a problem than to test for the platform
that has the problem.
Other platforms do have issues with some *rint() functions too IIRC
also most of the codecs wont be affected so hard failing in configure
is not very nice to the user



[...]

> -    av_assert0(LIBAVUTIL_VERSION_MICRO >= 100);
> -    av_assert0(HAVE_MMX2 == HAVE_MMXEXT);

These 2 lack any explanation of why they would be removed in the
commit message

[...]
-- 
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: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150320/9b317042/attachment.asc>


More information about the ffmpeg-devel mailing list