Re: [FFmpeg-devel] [PATCH v1 1/4] avcodec/webp: move definitions into header

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Thilo Borgmann:
> ---
>  libavcodec/webp.c | 17 +--------------
>  libavcodec/webp.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+), 16 deletions(-)
>  create mode 100644 libavcodec/webp.h
> 
> diff --git a/libavcodec/webp.c b/libavcodec/webp.c
> index d35cb66f8d..15152ec8fb 100644
> --- a/libavcodec/webp.c
> +++ b/libavcodec/webp.c
> @@ -52,22 +52,7 @@
>  #include "thread.h"
>  #include "tiff_common.h"
>  #include "vp8.h"
> -
> -#define VP8X_FLAG_ANIMATION             0x02
> -#define VP8X_FLAG_XMP_METADATA          0x04
> -#define VP8X_FLAG_EXIF_METADATA         0x08
> -#define VP8X_FLAG_ALPHA                 0x10
> -#define VP8X_FLAG_ICC                   0x20
> -
> -#define MAX_PALETTE_SIZE                256
> -#define MAX_CACHE_BITS                  11
> -#define NUM_CODE_LENGTH_CODES           19
> -#define HUFFMAN_CODES_PER_META_CODE     5
> -#define NUM_LITERAL_CODES               256
> -#define NUM_LENGTH_CODES                24
> -#define NUM_DISTANCE_CODES              40
> -#define NUM_SHORT_DISTANCES             120
> -#define MAX_HUFFMAN_CODE_LENGTH         15
> +#include "webp.h"
>  
>  static const uint16_t alphabet_sizes[HUFFMAN_CODES_PER_META_CODE] = {
>      NUM_LITERAL_CODES + NUM_LENGTH_CODES,
> diff --git a/libavcodec/webp.h b/libavcodec/webp.h
> new file mode 100644
> index 0000000000..90baa71182
> --- /dev/null
> +++ b/libavcodec/webp.h
> @@ -0,0 +1,55 @@
> +/*
> + * WebP image format definitions
> + * Copyright (c) 2020 Pexeso Inc.
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * WebP image format definitions.
> + */
> +
> +#ifndef AVCODEC_WEBP_H
> +#define AVCODEC_WEBP_H
> +
> +#define VP8X_FLAG_ANIMATION             0x02
> +#define VP8X_FLAG_XMP_METADATA          0x04
> +#define VP8X_FLAG_EXIF_METADATA         0x08
> +#define VP8X_FLAG_ALPHA                 0x10
> +#define VP8X_FLAG_ICC                   0x20
> +
> +#define ANMF_DISPOSAL_METHOD            0x01
> +#define ANMF_DISPOSAL_METHOD_UNCHANGED  0x00
> +#define ANMF_DISPOSAL_METHOD_BACKGROUND 0x01
> +
> +#define ANMF_BLENDING_METHOD            0x02
> +#define ANMF_BLENDING_METHOD_ALPHA      0x00
> +#define ANMF_BLENDING_METHOD_OVERWRITE  0x02
> +
> +#define MAX_PALETTE_SIZE                256
> +#define MAX_CACHE_BITS                  11
> +#define NUM_CODE_LENGTH_CODES           19
> +#define HUFFMAN_CODES_PER_META_CODE     5
> +#define NUM_LITERAL_CODES               256
> +#define NUM_LENGTH_CODES                24
> +#define NUM_DISTANCE_CODES              40
> +#define NUM_SHORT_DISTANCES             120
> +#define MAX_HUFFMAN_CODE_LENGTH         15
> +
> +
> +#endif /* AVCODEC_WEBP_H */

1. Some of these defines (like MAX_CACHE_BITS) are unused now and seem
to stay that way in your patchset.
2. If you move defines in a header, you need to ensure that they are
properly prefixed so that no conflicts can arise. This is particularly
true of defines like VP8X_FLAG_* whose name actually indicates that they
belong into a vp8.h.
3. It seems that your patchset only includes this header in webp.c; they
are not used outside of it. So there is no need for a header.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".