[vlc-devel] Several "mini include-guards" within `include/vlc_codecs.h`; rationale?

[Filip Roséen]

I am in the middle of a massive clean up of the codebase, currently
boiling down to removing usage of reserved identifiers (in order to make
the code compliant with the C (WG15) and C++ (WG21) Standard).

Partially disabling of header-content, why?
===========================================

During my quest I stumbled upon the below in `modules/codec/mft.c`:

    46: #define _VIDEOINFOHEADER_
    47: #include <vlc_codecs.h>

Inside `include/vlc_codecs.h` we find the following:

    155: #ifndef _VIDEOINFOHEADER_
    156: #define _VIDEOINFOHEADER_
    157: typedef struct
    158: ATTR_PACKED
    159: {
    160:     RECT32                  rcSource;
    161:     RECT32                  rcTarget;
    162:     uint32_t                dwBitRate;
    163:     uint32_t                dwBitErrorRate;
    164:     REFERENCE_TIME          AvgTimePerFrame;
    165:     VLC_BITMAPINFOHEADER    bmiHeader;
    166: } VIDEOINFOHEADER;
    167: #endif


It seems like the author (Felix Abecassis <felix.abecassis at gmail.com>
according to `git blame -e modules/codec/mft.c`) wanted to ignore the
`typedef` of `VIDEOINFOHEADER`. I have tried digging around but I have
yet to find a single reason for disabling that part of the header.

Questions
---------

  - Can the following patch be applied without causing issues?

        --- a/modules/codec/mft.c
        +++ b/modules/codec/mft.c
        @@ -43,7 +43,6 @@
         #include <vlc_plugin.h>
         #include <vlc_codec.h>
         #include "../packetizer/h264_nal.h"
        -#define _VIDEOINFOHEADER_
         #include <vlc_codecs.h>
         
         #include <mfapi.h>


-------------------------------------------------------------------------------


"Protected" regions within `include/vlc_codecs.h`
=================================================

I also noticed that `include/vlc_codecs.h` have a lot of other parts
that follow the same style as previously mentioned, such as:

    169: #ifndef _RGBQUAD_
    170: #define _RGBQUAD_
    171: typedef struct
    172: ATTR_PACKED
    173: {
    174:     uint8_t rgbBlue;
    175:     uint8_t rgbGreen;
    176:     uint8_t rgbRed;
    177:     uint8_t rgbReserved;
    178: } RGBQUAD1;
    179: #endif

At first I thought cases such as the above was a leftover from when
someone merged several headers into one, but when looking at the commit
log this does not seem to be the case.

According to the commit log, the code was introduced to `include/codecs.h` (Jan 2005),
and later that file was renamed to `include/vlc_codecs.h` (Nov 2006).

The above is based on the below:

    % git log -S "_RGBQUAD_"
    % git diff d3fe7f28797d4dba65ffcdd60bf932e758a48a9e\^ d3fe7f28797d4dba65ffcdd60bf932e758a48a9e
    % git diff c330b1e1ef13296994fd0e004d4af5286367319e\^ c330b1e1ef13296994fd0e004d4af5286367319e

Questions
---------

  - Are the weird "include-guards" in `include/vlc_codecs.h` documented
    behavior, or can they safely be removed (or atleast renamed to make
    the identifiers legal)?

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160220/f344f78f/attachment.html>