[vlc-devel] [PATCH] avi: avoid double definition on BI_* values

Alexandre Janniaux ajanni at videolabs.io
Sun Mar 29 15:04:56 CEST 2020


Hi,

> Can we be sure those have the right values always, on Win32?

Yes, we probably do, but having the defines for half the
platforms and something else for half the other is a bit
confusing and I'm not sure when they are available and
when they are not given because it seems to be documented
as defined by IDL files, not explicitly any header directly.
wingdi.h expose some of them, Windows Media Device Manager
seems to redeclare them too, etc...

It's also already not necessarily available depending on
the system, not that it matters a lot since XP, and the
different headers providing those values might not even
provide all of them.

Given that confusing situation, and as we already need
to define them half of the time, always defining them
was much simpler.

Regards,
--
Alexandre Janniaux
Videolabs

On Sat, Mar 28, 2020 at 10:39:00PM +0100, Marvin Scholz wrote:
> On 28 Mar 2020, at 19:17, Jean-Baptiste Kempf wrote:
>
> > Why not just not define them on Win32?
> >
>
> Can we be sure those have the right values always, on Win32?
>
> > On Fri, Mar 27, 2020, at 14:02, Alexandre Janniaux wrote:
> > > Those values are already defined on Windows, but never defined on
> > > other
> > > platforms. To keep the same behaviour everywhere, keep the defines
> > > but
> > > prefix them with VLC_*.
> > > ---
> > >  modules/demux/avi/bitmapinfoheader.h | 36
> > > ++++++++++++++--------------
> > >  modules/demux/avi/libavi.h           | 11 ---------
> > >  modules/mux/avi.c                    |  4 ++--
> > >  3 files changed, 20 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/modules/demux/avi/bitmapinfoheader.h
> > > b/modules/demux/avi/bitmapinfoheader.h
> > > index 3208a38d7b..68a8ae496a 100644
> > > --- a/modules/demux/avi/bitmapinfoheader.h
> > > +++ b/modules/demux/avi/bitmapinfoheader.h
> > > @@ -25,14 +25,14 @@
> > >  #include <vlc_codecs.h>
> > >  #include <limits.h>
> > >
> > > -/* biCompression / Others are FourCC */
> > > -#define BI_RGB              0x0000
> > > -#define BI_RLE8             0x0001
> > > -#define BI_RLE4             0x0002
> > > -#define BI_BITFIELDS        0x0003
> > > -#define BI_CMYK             0x000B
> > > -#define BI_CMYKRLE8         0x000C
> > > -#define BI_CMYKRLE4         0x000D
> > > +///* biCompression / Others are FourCC */
> > > +#define VLC_BI_RGB              0x0000
> > > +#define VLC_BI_RLE8             0x0001
> > > +#define VLC_BI_RLE4             0x0002
> > > +#define VLC_BI_BITFIELDS        0x0003
> > > +#define VLC_BI_CMYK             0x000B
> > > +#define VLC_BI_CMYKRLE8         0x000C
> > > +#define VLC_BI_CMYKRLE4         0x000D
> > >
> > >  static const struct
> > >  {
> > > @@ -105,8 +105,8 @@ static inline int ParseBitmapInfoHeader(
> > > VLC_BITMAPINFOHEADER *p_bih, size_t i_b
> > >      else
> > >          i_bihextra = 0;
> > >
> > > -    if( p_bih->biCompression == BI_RGB ||
> > > -        p_bih->biCompression == BI_BITFIELDS )
> > > +    if( p_bih->biCompression == VLC_BI_RGB ||
> > > +        p_bih->biCompression == VLC_BI_BITFIELDS )
> > >      {
> > >          switch( p_bih->biBitCount )
> > >          {
> > > @@ -141,7 +141,7 @@ static inline int ParseBitmapInfoHeader(
> > > VLC_BITMAPINFOHEADER *p_bih, size_t i_b
> > >                  break;
> > >          }
> > >
> > > -        if( p_bih->biCompression == BI_BITFIELDS ) /* Only 16 & 32
> > > */
> > > +        if( p_bih->biCompression == VLC_BI_BITFIELDS ) /* Only 16 &
> > > 32
> > > */
> > >          {
> > >              if( i_bihextra >= 3 * sizeof(uint32_t) )
> > >              {
> > > @@ -226,28 +226,28 @@ static inline VLC_BITMAPINFOHEADER *
> > > CreateBitmapInfoHeader( const es_format_t *
> > >      {
> > >          case VLC_CODEC_RGB32:
> > >              biBitCount = 32;
> > > -            biCompression = MatchBitmapRGBMasks( fmt ) ? BI_RGB :
> > > BI_BITFIELDS;
> > > +            biCompression = MatchBitmapRGBMasks( fmt ) ? VLC_BI_RGB
> > > :
> > > VLC_BI_BITFIELDS;
> > >              break;
> > >          case VLC_CODEC_BGRA:
> > >          case VLC_CODEC_RGBA:
> > >          case VLC_CODEC_ARGB:
> > >              biBitCount = 32;
> > > -            biCompression = MatchBitmapRGBMasks( fmt ) ? BI_RGB :
> > > BI_BITFIELDS;
> > > +            biCompression = MatchBitmapRGBMasks( fmt ) ? VLC_BI_RGB
> > > :
> > > VLC_BI_BITFIELDS;
> > >              b_has_alpha = true;
> > >              break;
> > >          case VLC_CODEC_RGB24:
> > >              biBitCount = 24;
> > > -            biCompression = BI_RGB;
> > > +            biCompression = VLC_BI_RGB;
> > >              break;
> > >          case VLC_CODEC_RGB16:
> > >          case VLC_CODEC_RGB15:
> > >              biBitCount = 16;
> > > -            biCompression = BI_BITFIELDS;
> > > +            biCompression = VLC_BI_BITFIELDS;
> > >              break;
> > >          case VLC_CODEC_RGBP:
> > >          case VLC_CODEC_GREY:
> > >              biBitCount = 8;
> > > -            biCompression = BI_RGB;
> > > +            biCompression = VLC_BI_RGB;
> > >              break;
> > >          case VLC_CODEC_MP4V:
> > >              biCompression = VLC_FOURCC( 'X', 'V', 'I', 'D' );
> > > @@ -260,7 +260,7 @@ static inline VLC_BITMAPINFOHEADER *
> > > CreateBitmapInfoHeader( const es_format_t *
> > >
> > >      size_t i_bih_extra = 0;
> > >      size_t i_bmiColors = 0;
> > > -    if( biCompression == BI_BITFIELDS )
> > > +    if( biCompression == VLC_BI_BITFIELDS )
> > >          i_bmiColors = (b_has_alpha) ? 16 : 12;
> > >      else if ( fmt->i_codec == VLC_CODEC_RGBP )
> > >          i_bmiColors = fmt->video.p_palette ?
> > > (fmt->video.p_palette->i_entries * 4) : 0;
> > > @@ -275,7 +275,7 @@ static inline VLC_BITMAPINFOHEADER *
> > > CreateBitmapInfoHeader( const es_format_t *
> > >      uint8_t *p_bih_extra = (uint8_t *) &p_bih[1];
> > >      uint8_t *p_bmiColors = p_bih_extra + i_bih_extra;
> > >      p_bih->biClrUsed = 0;
> > > -    if( biCompression == BI_BITFIELDS )
> > > +    if( biCompression == VLC_BI_BITFIELDS )
> > >      {
> > >          SetDWBE( &p_bmiColors[0], fmt->video.i_rmask );
> > >          SetDWBE( &p_bmiColors[4], fmt->video.i_gmask );
> > > diff --git a/modules/demux/avi/libavi.h b/modules/demux/avi/libavi.h
> > > index 731fb2c1c7..7aa4997978 100644
> > > --- a/modules/demux/avi/libavi.h
> > > +++ b/modules/demux/avi/libavi.h
> > > @@ -19,17 +19,6 @@
> > >   * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> > >
> > > *****************************************************************************/
> > >
> > > -/* biCompression / Others are FourCC */
> > > -#define BI_RGB              0x0000
> > > -#define BI_RLE8             0x0001
> > > -#define BI_RLE4             0x0002
> > > -#define BI_BITFIELDS        0x0003
> > > -#define BI_JPEG             0x0004
> > > -#define BI_PNG              0x0005
> > > -#define BI_CMYK             0x000B
> > > -#define BI_CMYKRLE8         0x000C
> > > -#define BI_CMYKRLE4         0x000D
> > > -
> > >  /* flags for use in <dwFlags> in AVIFileHdr */
> > >  #define AVIF_HASINDEX       0x00000010  /* Index at end of file? */
> > >  #define AVIF_MUSTUSEINDEX   0x00000020
> > > diff --git a/modules/mux/avi.c b/modules/mux/avi.c
> > > index d6e1b0e020..f635718061 100644
> > > --- a/modules/mux/avi.c
> > > +++ b/modules/mux/avi.c
> > > @@ -471,9 +471,9 @@ static int PrepareSamples( const avi_stream_t
> > > *p_stream,
> > >         }
> > >      }
> > >
> > > -    /* RV24 is only BGR in AVI, and we can't use BI_BITFIELD */
> > > +    /* RV24 is only BGR in AVI, and we can't use VLC_BI_BITFIELD */
> > >      if( p_stream->i_cat == VIDEO_ES &&
> > > -        p_stream->p_bih->biCompression == BI_RGB &&
> > > +        p_stream->p_bih->biCompression == VLC_BI_RGB &&
> > >          p_stream->p_bih->biBitCount == 24 &&
> > >          (p_fmt->video.i_bmask != 0xFF0000 ||
> > >           p_fmt->video.i_rmask != 0x0000FF) )
> > > --
> > > 2.26.0
> > >
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> >
> > --
> > Jean-Baptiste Kempf -  President
> > +33 672 704 734
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list