[Speex-dev] Strange style of includes

[Jean-Marc Valin]

FYI, gtk includes headers exactly like Speex does. Of course, so does 
libc. In any case, I'd like to know how you'd include them anyway.

	Jean-Marc

On 10-06-21 09:59 PM, Pavel Pavlov wrote:
>> -----Original Message-----
>> From: Jean-Marc Valin [mailto:jean-marc.valin at usherbrooke.ca]
>> Hi
>>
>> On 10-06-21 07:24 PM, Pavel Pavlov wrote:
>>> I'm just curious, who and why came up with that strange way to write includes:
>>
>> I did. Because I think that's the way to go.
>
> I'm just curios, why? Is there some compiler/platform that requires it to be this way? I didn't see any project that does that. Ffmpeg, for instance, has huge user base and most of its headers are included this way by user programs:
> #include<libavcodec/avcodec.h>
> But internally, you can't find a single place where ffmpeg includes its own headers using angle brackets. The only headers that are included this way are external and system includes.
>
>>
>>> #ifdef _BUILD_SPEEX
>>> # include "speex_types.h"
>>> #else
>>> # include<speex/speex_types.h>
>>> #endif
>>>
>>> I personally consider it a bug. If I use speex then obviously I should not have
>> _BUILD_SPEEX defined.
>>
>> So far so good.
>>
>>> The broken behavior happens in this simple scenario:
>>> 1) I check out latest git version to ./src/speex
>>> 2) My system has an older "release" version installed to regular install location.
>>
>> So problem so far.
>>
>>> 3) I want to use the version I just checked out, I include filed this way:
>> "...whatever/src/speex/speex_bits.h"
>>
>> Wrong! What you need to do is add "../whatever/src" to your include path (e.g.
>> -I with gcc)
>>
>
> I don't see a reason to add anything to the include path, it just doesn't make sense and nothing on the face on the planet can convince me otherwise. Look, I need to #include "../whatever/src/headefile.h", why is that I need to do something else, if #include "../whatever/src/headefile.h" does what exactly what I need? If #include "../whatever/src/headefile.h" produces an error, then "../whatever/src/headefile.h" has a problem. Speex doesn't have multiple levels of subprojects that include eachothers' header files (eg. In ffmpeg, avcodec uses avutil etc), and therefore speex doesn't need to include headers from different folders, they are all in the same folder and there is no reason to include them that strange way. If SPEEX had mulple subprojects (like speexdsp, speexutils etc) then to use some speex a programmer would have to add include search path to find speex, but still there is no reason for the speexdsp and speexutil to include each other's headers using<>!
>
> I'm not sure how much you care about ms compiler, but it will look in system path for angle bracket includes.
> There was some problem with gcc and in late version they changed that behavior as well to be able specify system include search path and -iquote search path. (and I believe that change originated from that issue that Symbian's port of gcc had!)
> http://gcc.gnu.org/onlinedocs/cpp/Search-Path.html :
> "For example, if /usr/include/sys/stat.h  contains #include "types.h", GCC looks for types.h first in /usr/include/sys, then in its usual search path.". MS compiler has exactly the same behavior, so I still don't understand what's the reason to use<>  if simple include "..." will include a header from the same folder (exactly what it needs to do)??
>
>>> Broken behavior: instead of using files from check out version, speex
>>> includes will include system installed versions of old speex! That's
>>> just bad, I've never seen anything like that
>>
>> Fix your code -- I've never seen anything like that ;-)
>>
>
> My code has no problems, I removed that _BUILD_SPEEX thing ;)
>
>>> I'll attempt to answer the "why" question... speex also has Symbian port, and
>> just like everything is ugly crap in Symbian world, their version of gcc is also
>> broken and you can't include headers relative to the directory of current file and
>> that approach with<path/to/file.h>   would probably fix that problem in Symbian.
>> I think _build_speex thing should be removed, plus have_config_h usually has
>> the same meaning: to indicate that the library is being build and not used.
>>
>> I think you managed to get it wrong all the way. FYI, I had nothing to do with the
>> Symbian port and had no idea that Symbian had include issues.
>
> Symbian on its own has no include issues. The GCC build that distributed with their sdk has some horrible problem, so that you can't #include "folder/file.h" unless . is in the include search path. So, big projects that have many levels and folders of files end up with Symbian build files that contain every single directory in the list of includes!
>
>>
>>> Speex code uses EXPORT to mark public api functions. If default
>>> visibility is hidden then it's all ok,
>>
>> Exactly. That's the idea.
>>
>>> but I'd personally use static for private functions if they aren't referenced
>> across different files.
>>
>> Yes, I know what "static" is for. If I forgot functions that should be static, you
>> can send a patch.
>>
>>> I also use one file compilation: libspeex.c that includes all of speex so that
>> speex can be used in any library by simply adding reference to that .c file to a
>> makefile (just like pthreads does). The is a minor problem though:  nb_celp.c and
>> sb_celp.c redefine the same macro with different values: LSP_MARGIN,
>> LSP_DELTA1, LSP_DELTA2 (which result in a warning) and _kiss_fft_guts.h
>> doesn't have header guards (results in errors).
>>
>> kiss_fft could probably use some include guards. Other than that, if you want to
>> do things like include all .c in the same file, just #undef the what you need.
>>
>
> Well, that's exactly what I did.
>
>
> What about all these patents that cover amr and g72X codecs. Are there many of them? Since speex doesn't use these algorithms (like acelp) does it mean that it's inferior to amr for example?
>
>