[Ffmpeg-devel] [PATCH] Dead code cleanup - C++

[Yuri Vilmanis]

Removed some dead code relating to C++. As no part of the codebase will ever be seen by a C++ compiler, any code protected by #ifdef__cplusplus will *never* get past the preprocessor, and so can be safely removed. The files in which these guards appear are not valid C++ anyway, so removing these references to C++ should reduce future confusion on this issue. The C++ wrapper "fobs" (or other C++ wrappers I'm not aware of) can be used by anyone requiring C++ support.

- Yuri 

P.S.

For the record, I prefer C over C++ for this kind of work (i.e. anything that really wouldn't benefit from OO & templates), love the expressional power of some of the new C99 constructs, write all my own C code for gcc, and don't generally lay eyes on Windows outside of office hours. But I stand behind the points I made in my previous email, which I believe were technically correct (apart from one regarding linkage, below - well spotted, Michael), properly referenced, and with appropriate qualifiers. Replies to some comments follow, but first, a reading from the scriptures (or as close as I could find):

>From the FFMPEG F.A.Q. http://ffmpeg.mplayerhq.hu/faq.html :

2.4 Can you support my C compiler XXX ?
No. Only GCC is supported. GCC is ported to most systems available and there is no need to pollute the source code with #ifdefs related to the compiler.

2.6 Can you add automake, libtool or autoconf support ?
No. These tools are too bloated and they complicate the build. Moreover, since only `gcc' is supported they would add little advantages in terms of portability. 

and

2.7 Why not rewrite ffmpeg in object-oriented C++ ?
ffmpeg is already organized in a highly modular manner and does not need to be rewritten in a formal object language. Further, many of the developers favor straight C; it works for them. For more arguments on this matter, read "Programming Religion" at (http://lkml.org/faq/lkmlfaq-15.html).

And lets hear someone argue that #ifdefs for another, unsupported language are 
not #ifdefs related to the compiler.

OK, I know its rude to quote someone's own FAQ to them, sorry...

>ffmpeg is in C, just C, if you want C++ use the wrapper (there is one).
>Luca Barbato

>We're not interested.  Ranting doesn't help.
>M?ns Rullg?rd

As stated, the intention of the previously submitted patch is not to have it adopted added to ffmpeg svn, it is merely there to satisfy the license agreement (distrubuting the source code of a modified version). This patch was made to meet a particular goal (linking against a 64 bit windows binary) which was, unfortunately, not possible using other methods (e.g. compiling from gcc, which was actuall my preferred option - does anyone here think that porting C99 code to Visual C is *fun*?). C++ wrappers wont help with this.

By the way, I dont think I was ranting, I simply added some interesting references to the gcc website and some ISO standards to the end of my post, and a short discussion of their ramifications, with some conclusions derived from those references. I know this is not common practice, if it came across as a rant, I apologize.

>Not true, this project is not supposed to be gcc-only.
>Diego

Read your own F.A.Q., section 2.4 & 2.6 (see above). Or clarify it, which may be a better option...

>i fear that the fact that ffmpeg works with gcc mostly invalidates your
>claim that they are broken with gcc
>
>iam not aware of any bugreports about this, not to mention iam not aware
>of any difference betweem gcc and C99 about "static inline" care to
>elaborate on this? we dont use non static inline or extern inline or anything
>like that ...

Some non-static inlines, According To grep:
ffmpeg/libswscale/swscale_template.c:2663:inline static void RENAME(hcscale)(uint16_t *dst, long dstWidth, uint8_t *src1, uint8_t *src2,
ffmpeg/libavcodec/motion_est_template.c:236:inline int ff_get_mb_score(MpegEncContext * s, int mx, int my, int src_index,
ffmpeg/libavcodec/motion_est_template.c:1106:inline int ff_epzs_motion_search(MpegEncContext * s, int *mx_ptr, int *my_ptr,
ffmpeg/libavcodec/xvmcvideo.c:47:inline void XVMC_init_block(MpegEncContext *s){
ffmpeg/libavcodec/mpegvideo.c:3331:inline int ff_h263_round_chroma(int x){
ffmpeg/libavcodec/mpegvideo.h:766:inline int ff_epzs_motion_search(MpegEncContext * s, int *mx_ptr, int *my_ptr,
ffmpeg/libavcodec/mpegvideo.h:769:inline int ff_get_mb_score(MpegEncContext * s, int mx, int my, int src_index,

I'll let you fix em, seeing as how they dont exist (I'll just take your word on that).

You seem to know what you're talking about... I confess I haven't personally tested all these against the standard myself, I can only go on the information available. On http://gcc.gnu.org/c99status.html, under the heading "Further notes" there is a dot point which reads

"C99 inline functions do not generate an external definition if declared without extern, but do if declared with extern, the opposite of GCC's handling of inline and extern inline. This will probably require existing glibc headers to be fixincluded."

This and other apparent errors I pointed out in the supported feature table on the said page should probably be brought to the attention of the gcc website maintainers. I'll leave that to someone better versed in compiler testing than I, maybe you would do the honours? (Yes, I'm serious, if the gcc website is in error, someone should do something about it, and if you can tell where the errors are, you can do a better job of it than I)

>could you please also tell me which pascal and fortran rules we violate
>no we really DONT support c++ in any way, if it happens to work then well
>be happy, if some small and clean patch would help you that might be
>considered for svn but beyond that i fear we cant help you

All of em ;) but your headers don't contain anything which implies you might be trying to support pascal or fortran, so theres less chance of poor sods such as I becoming confused on the issue. Unlike a few carelessly scattered copies of #ifdef __cplusplus.

>do you know why everyone who uses visual <bluescreen, restarts system) C++
>has a problem with understanding the different between standart and gcc?
>we HATE gcc and we certainly are not, where not and never will be
>gnucentric, ffmpeg code is stanard C, everything else is under #ifdefs
>like asm, always_inline and so on
>it must be something like
>* user sees vc++ fail with random program
>* user sees gcc to suceed with same random program
>* user concludes that program is written specifically for gcc as he knows
>  MS always follows all standards very carefully
> ...
>right after you wrap your head(er) in
>#ifdef VISSUAL_C_FANATIC

On the question of whether FFMpeg supports compilers other than gcc, read your own F.A.Q., section 2.4 & 2.6 (see above).

Also, note that there is indeed an ISO standard, number 14882, which comes in two editions, 1998 (first edition) and 2003 (second edition), which defines what is and is not valid C++ (in headers or elsewhere), and in particular, allowed reliance on the standard C libraries is defined in Annex C.2. I didn't write it, I just happen to have read it.

Assuming that FFMpeg does not support linkage from C++ programs, as seems to be the generally adopted position (and is a reasonable enough thing), then yes, inttypes.h is indeed a standard C header, and A-OK with the world. Though I still think stdint.h would be a better choice here, as the whole functionality provided by inttypes.h according to the C99 standard is not required from the public interface of ffmpeg... but thats really more of a style issue.

Finally, when you invoke the standards, please be so kind to specify which standard, what language it applies to, and where in that standard confirmation can be found for the point you're trying to make (chapter and paragraph numbers are often provided in international standards, for just this kind of thing!). Otherwise, I'm forced to assume you're following a similar method of testing standards compliance to the one you attribute to me. Making cliche nonsense putdown comments about a compiler you dont like at the same time (however justified your dislike may be) just adds to the impression.

>> A third option is simply to drop support for C++ linkage - the 'pure C'
>we NEVER did support that so how could we drop it?!
> Michael

Ah. My wording seems to have been a bit off there... I actually meant "support for C++ source code to include ffmpeg headers and use that functionality with ffmpeg binaries compiled using a C compiler, through the use of the C++ extern "C" linkage specifier to specify C linkage, in the header files". I thought what I wrote would be understood, but what I wrote was, technically, the opposite of what I meant. Sorry.

>Actually you are wrong. The second part (ugliness) is and always was the
>only reason to reject things. Which, regardless of standards, is why
>assuming that a working inttypes.h exists is the solution that wins
>hands down (for reference, MPlayer did this since loong times and even
>shipped with an inttypes.h - but it was such a basic implementation that
>nowadays doing this is considered harmful).

Fair enough. w.r.t. the gcc-only assumption, I'm just going by the FAQ and comments made by *some* developers on this list (you may have to dig thru the archives a bit, but they're there). Probably time for someone to update the FAQ. As for including an inttypes.h, I agree, this is a C99 header and should be part of your C compiler environment; its only a problem with C++. As regards standards, any given piece of code either does or does not comply, if it relies on something not in the standard which obviously would be if it were indtended to be, it does not comply - as in the case of inttypes.h and C++.

>Now most of what you say following is unfortunately irrelevant since
>ffmpeg is _not_ a C++ project, but a C one, and (almost?) all you say
>below refers to C++, those two have nothing to do with each other (or at
>least not much more that any other two random programming languages).

As I said, if you dont intend to support C++, thats fine - in which case the source code is currently a bit misleading, and the attached patch remedies this.

>And one comment: I really, really can't imagine your NaN comparisons are
>the best way. At least use uint64_t, that saves you one comparison (and
>64 bit types are even supported in compilers for 8 bit microcontrollers,
>so I can't imagine there is any compiler lacking them). It probably is
>broken anyway though, since floating point and integer endianness can
>differ, so you might consider using some of the stuff from
>libavutil/intfloat_readwrite (in case you even care though *g*).
>Greetings,
>Reimar D?ffinger

Thanks, I'll look into it - as I said, that was a dirty, inelegant patch created for a very specific need, and any hints and improvements are welcome.

-Yuri
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cplusplus_cleanup.diff
Type: text/x-diff
Size: 2540 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070126/70c73361/attachment.diff>