James Almer:
> They don't belong anywhere in particular, so move them to a new generic header
> called defs.h, following the same idea as the one from lavc.
>
> Signed-off-by: James Almer <jamr...@gmail.com>
> ---
> libavutil/avutil.h | 94 +++-------------------------
> libavutil/defs.h | 149 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 159 insertions(+), 84 deletions(-)
> create mode 100644 libavutil/defs.h
>
> diff --git a/libavutil/avutil.h b/libavutil/avutil.h
> index ee709fbb2a..1cf562c73f 100644
> --- a/libavutil/avutil.h
> +++ b/libavutil/avutil.h
> @@ -191,100 +191,36 @@ const char *avutil_license(void);
> * @}
> */
>
> +#include "common.h"
> +#include "defs.h"
> +#include "rational.h"
> +#include "version.h"
> +#include "macros.h"
> +#include "mathematics.h"
> +#include "log.h"
> +#include "pixfmt.h"
> +
> /**
> * @addtogroup lavu_media Media Type
> * @brief Media Type
> + * @{
> */
>
> -enum AVMediaType {
> - AVMEDIA_TYPE_UNKNOWN = -1, ///< Usually treated as AVMEDIA_TYPE_DATA
> - AVMEDIA_TYPE_VIDEO,
> - AVMEDIA_TYPE_AUDIO,
> - AVMEDIA_TYPE_DATA, ///< Opaque data information usually
> continuous
> - AVMEDIA_TYPE_SUBTITLE,
> - AVMEDIA_TYPE_ATTACHMENT, ///< Opaque data information usually sparse
> - AVMEDIA_TYPE_NB
> -};
> -
> /**
> * Return a string describing the media_type enum, NULL if media_type
> * is unknown.
> */
> const char *av_get_media_type_string(enum AVMediaType media_type);
>
> -/**
> - * @defgroup lavu_const Constants
> - * @{
> - *
> - * @defgroup lavu_enc Encoding specific
> - *
> - * @note those definition should move to avcodec
> - * @{
> - */
> -
> -#define FF_LAMBDA_SHIFT 7
> -#define FF_LAMBDA_SCALE (1<<FF_LAMBDA_SHIFT)
> -#define FF_QP2LAMBDA 118 ///< factor to convert from H.263 QP to lambda
> -#define FF_LAMBDA_MAX (256*128-1)
> -
> -#define FF_QUALITY_SCALE FF_LAMBDA_SCALE //FIXME maybe remove
> -
> /**
> * @}
> - * @defgroup lavu_time Timestamp specific
> - *
> - * FFmpeg internal timebase and timestamp definitions
> - *
> - * @{
> - */
> -
> -/**
> - * @brief Undefined timestamp value
> - *
> - * Usually reported by demuxer that work on containers that do not provide
> - * either pts or dts.
> */
>
> -#define AV_NOPTS_VALUE ((int64_t)UINT64_C(0x8000000000000000))
> -
> /**
> - * Internal time base represented as integer
> - */
> -
> -#define AV_TIME_BASE 1000000
> -
> -/**
> - * Internal time base represented as fractional value
> - */
> -
> -#ifdef __cplusplus
> -/* ISO C++ forbids compound-literals. */
> -#define AV_TIME_BASE_Q av_make_q(1, AV_TIME_BASE)
> -#else
> -#define AV_TIME_BASE_Q (AVRational){1, AV_TIME_BASE}
> -#endif
> -
> -/**
> - * @}
> - * @}
> * @defgroup lavu_picture Image related
> - *
> - * AVPicture types, pixel formats and basic image planes manipulation.
> - *
> * @{
> */
>
> -enum AVPictureType {
> - AV_PICTURE_TYPE_NONE = 0, ///< Undefined
> - AV_PICTURE_TYPE_I, ///< Intra
> - AV_PICTURE_TYPE_P, ///< Predicted
> - AV_PICTURE_TYPE_B, ///< Bi-dir predicted
> - AV_PICTURE_TYPE_S, ///< S(GMC)-VOP MPEG-4
> - AV_PICTURE_TYPE_SI, ///< Switching Intra
> - AV_PICTURE_TYPE_SP, ///< Switching Predicted
> - AV_PICTURE_TYPE_BI, ///< BI type
> -};
> -
> /**
> * Return a single letter to describe the given picture type
> * pict_type.
> @@ -298,14 +234,6 @@ char av_get_picture_type_char(enum AVPictureType
> pict_type);
> * @}
> */
>
> -#include "common.h"
> -#include "rational.h"
> -#include "version.h"
> -#include "macros.h"
> -#include "mathematics.h"
> -#include "log.h"
> -#include "pixfmt.h"
> -
> /**
> * Return x default pointer in case p is NULL.
> */
> @@ -343,8 +271,6 @@ unsigned av_int_list_length_for_size(unsigned elsize,
> */
> AVRational av_get_time_base_q(void);
>
> -#define AV_FOURCC_MAX_STRING_SIZE 32
> -
Splitting AV_FOURCC_MAX_STRING_SIZE from av_fourcc2str() makes no sense.
> #define av_fourcc2str(fourcc)
> av_fourcc_make_string((char[AV_FOURCC_MAX_STRING_SIZE]){0}, fourcc)
>
> /**
> diff --git a/libavutil/defs.h b/libavutil/defs.h
> new file mode 100644
> index 0000000000..9e9bb8bcc5
> --- /dev/null
> +++ b/libavutil/defs.h
> @@ -0,0 +1,149 @@
> +/*
> + * 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
> + */
> +
> +#ifndef AVUTIL_DEFS_H
> +#define AVUTIL_DEFS_H
> +
> +/**
> + * @file
> + * @ingroup lavu
> + * Misc types and constants that do not belong anywhere else.
> + */
> +
> +#include <stdint.h>
> +#include <stdlib.h>
stdlib for what?
> +
> +/**
> + * @addtogroup lavu_misc Other
> + * @{
> + *
> + * @defgroup preproc_misc Preprocessor String Macros
> + *
> + * @{
> + *
> + * @}
> + *
> + * @defgroup version_utils Library Version Macros
> + *
> + * @{
> + *
> + * @}
> + */
> +
> +/**
> + * @addtogroup lavu_media Media Type
> + * @{
> + */
> +enum AVMediaType {
> + AVMEDIA_TYPE_UNKNOWN = -1, ///< Usually treated as AVMEDIA_TYPE_DATA
> + AVMEDIA_TYPE_VIDEO,
> + AVMEDIA_TYPE_AUDIO,
> + AVMEDIA_TYPE_DATA, ///< Opaque data information usually
> continuous
> + AVMEDIA_TYPE_SUBTITLE,
> + AVMEDIA_TYPE_ATTACHMENT, ///< Opaque data information usually sparse
> + AVMEDIA_TYPE_NB
> +};
> +
> +/**
> + * @}
> + */
> +
> +/**
> + * @defgroup lavu_const Constants
> + * @{
> + *
> + * @defgroup lavu_enc Encoding specific
> + *
> + * @note those definition should move to avcodec
> + * @{
> + */
> +
> +#define FF_LAMBDA_SHIFT 7
> +#define FF_LAMBDA_SCALE (1<<FF_LAMBDA_SHIFT)
> +#define FF_QP2LAMBDA 118 ///< factor to convert from H.263 QP to lambda
> +#define FF_LAMBDA_MAX (256*128-1)
> +
> +#define FF_QUALITY_SCALE FF_LAMBDA_SCALE //FIXME maybe remove
> +
There is a comment that this stuff should be moved to avcodec (which you
move, too). Maybe it should be moved to avcodec instead of to this new
header? Do all of these constants even need to be public?
> +#define AV_FOURCC_MAX_STRING_SIZE 32
> +
> +/**
> + * @}
> + * @defgroup lavu_time Timestamp specific
> + *
> + * FFmpeg internal timebase and timestamp definitions
> + *
> + * @{
> + */
> +
> +/**
> + * @brief Undefined timestamp value
> + *
> + * Usually reported by demuxer that work on containers that do not provide
> + * either pts or dts.
> + */
> +
> +#define AV_NOPTS_VALUE ((int64_t)UINT64_C(0x8000000000000000))
> +
> +/**
> + * Internal time base represented as integer
> + */
> +
> +#define AV_TIME_BASE 1000000
> +
> +/**
> + * Internal time base represented as fractional value
> + */
> +
> +#ifdef __cplusplus
> +/* ISO C++ forbids compound-literals. */
> +#define AV_TIME_BASE_Q av_make_q(1, AV_TIME_BASE)
> +#else
> +#define AV_TIME_BASE_Q (AVRational){1, AV_TIME_BASE}
This would need an inclusion of rational.h for the user to use it. Is it
intended for public macros to not be immediately usable?
> +#endif
> +
> +/**
> + * @}
> + * @}
> + */
> +
> +/**
> + * @addtogroup lavu_picture Image related
> + *
> + * AVPicture types, pixel formats and basic image planes manipulation.
It is only AVPicture type.
> + *
> + * @{
> + */
> +
> +enum AVPictureType {
> + AV_PICTURE_TYPE_NONE = 0, ///< Undefined
> + AV_PICTURE_TYPE_I, ///< Intra
> + AV_PICTURE_TYPE_P, ///< Predicted
> + AV_PICTURE_TYPE_B, ///< Bi-dir predicted
> + AV_PICTURE_TYPE_S, ///< S(GMC)-VOP MPEG-4
> + AV_PICTURE_TYPE_SI, ///< Switching Intra
> + AV_PICTURE_TYPE_SP, ///< Switching Predicted
> + AV_PICTURE_TYPE_BI, ///< BI type
> +};
This splits AVPictureType from av_get_picture_type_char() which makes
little sense.
> +
> +/**
> + * @}
> + * @}
> + */
> +
> +#endif // AVUTIL_DEFS_H
Apart from that: This patch as-is would also need to add defs.h to the
list of installed header in the Makefile.
But actually, I don't really see the point of this patch. The problem of
avutil.h is not that it contains too much, but that it includes so much.
And fixing this runs into the problem that avutil.h is advertised to be
a "convenience header that includes libavutil's core" (which means:
users are allowed to rely on implicit inclusions). Which IMO should be
common.h, with avutil.h being a normal 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-requ...@ffmpeg.org with subject "unsubscribe".
