[FFmpeg-devel] [PATCH 02/11] avcodec/raw: Reduce number of avpriv symbols
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Mon Dec 20 17:03:54 EET 2021
Andreas Rheinhardt:
> libavcodec currently exports four avpriv symbols that deal with
> PixelFormatTags: avpriv_get_raw_pix_fmt_tags, avpriv_find_pix_fmt,
> avpriv_pix_fmt_bps_avi and avpriv_pix_fmt_bps_mov. The latter two are
> lists of PixelFormatTags, the former returns such a list and the second
> searches a list for a pixel format that matches a given fourcc; only
> one of the aforementioned three lists is ever searched.
>
> Yet for avpriv_pix_fmt_bps_avi, avpriv_pix_fmt_bps_mov and
> avpriv_find_pix_fmt the overhead of exporting these functions actually
> exceeds the size of said objects (at least for ELF; the following numbers
> are for x64 Ubuntu 20.10):
> The code size of avpriv_find_pix_fmt is small (GCC 10.2 37B, Clang 11 41B),
> yet exporting it adds a 20B string for the name alone to the exporting
> as well as to each importing library; there is more: Four bytes in the
> exporting libraries .gnu.hash; two bytes each for the exporting as well
> as each importing libraries .gnu.version; 24B in the exporting as well
> as each importing libraries .dynsym; 16B+24B for an entry in .plt as
> well as the accompanying relocation entry in .rela.plt for each
> importing library.
>
> The overhead for the lists is similar: The strings are 23B and the
> .plt+.rela.plt pair is replaced by 8B+24B for an entry in .got and
> a relocation entry in .rela.dyn. These lists have a size of 80 resp.
> 72 bytes.
>
> Yet for ff_raw_pix_fmt_tags, exporting it is advantageous compared to
> duplicating it into libavformat and potentially libavdevice. Therefore
> this commit replaces all library uses of the four symbols with a single
> function that is exported for shared builds. It has an enum parameter
> to choose the desired list besides the parameter for the fourcc. New
> lists can be supported with new enum values.
>
> Unfortunately, avpriv_get_raw_pix_fmt_tags could not be removed, as the
> fourcc2pixfmt tool uses the table of raw pix fmts. No other user of this
> function remains.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
> 1. It would make sense to deduplicate avpriv_pix_fmt_bps_(mov|avi)
> size-wise if it is preferred to keep the lists accessible to users.
> 2. One could handle the fourcc2pixfmt case in avpriv_pix_fmt_find(),
> too, if one added another parameter (say count). In this case
> avpriv_pix_fmt_find() will return the first pixfmt with the desired
> fourcc if count is zero, the second one is count is 1 etc. (and
> AV_PIX_FMT_NONE in case there is none any more). This would also make
> this function more future-proof.
>
> libavcodec/raw.c | 40 +++++++++++++++++++++++++++++++++++-----
> libavcodec/raw.h | 13 +++++++------
> libavcodec/rawdec.c | 8 ++++----
> libavcodec/utils.c | 11 -----------
> libavdevice/dshow.c | 2 +-
> libavformat/avienc.c | 2 +-
> libavformat/demux.c | 2 +-
> libavformat/movenc.c | 2 +-
> 8 files changed, 50 insertions(+), 30 deletions(-)
>
> diff --git a/libavcodec/raw.c b/libavcodec/raw.c
> index 079d5c5d10..5efc1eb465 100644
> --- a/libavcodec/raw.c
> +++ b/libavcodec/raw.c
> @@ -28,7 +28,7 @@
> #include "raw.h"
> #include "libavutil/common.h"
>
> -const PixelFormatTag ff_raw_pix_fmt_tags[] = {
> +static const PixelFormatTag raw_pix_fmt_tags[] = {
> { AV_PIX_FMT_YUV420P, MKTAG('I', '4', '2', '0') }, /* Planar formats */
> { AV_PIX_FMT_YUV420P, MKTAG('I', 'Y', 'U', 'V') },
> { AV_PIX_FMT_YUV420P, MKTAG('y', 'v', '1', '2') },
> @@ -299,12 +299,12 @@ const PixelFormatTag ff_raw_pix_fmt_tags[] = {
>
> const struct PixelFormatTag *avpriv_get_raw_pix_fmt_tags(void)
> {
> - return ff_raw_pix_fmt_tags;
> + return raw_pix_fmt_tags;
> }
>
> unsigned int avcodec_pix_fmt_to_codec_tag(enum AVPixelFormat fmt)
> {
> - const PixelFormatTag *tags = ff_raw_pix_fmt_tags;
> + const PixelFormatTag *tags = raw_pix_fmt_tags;
> while (tags->pix_fmt >= 0) {
> if (tags->pix_fmt == fmt)
> return tags->fourcc;
> @@ -313,7 +313,7 @@ unsigned int avcodec_pix_fmt_to_codec_tag(enum AVPixelFormat fmt)
> return 0;
> }
>
> -const PixelFormatTag avpriv_pix_fmt_bps_avi[] = {
> +static const PixelFormatTag pix_fmt_bps_avi[] = {
> { AV_PIX_FMT_PAL8, 1 },
> { AV_PIX_FMT_PAL8, 2 },
> { AV_PIX_FMT_PAL8, 4 },
> @@ -326,7 +326,7 @@ const PixelFormatTag avpriv_pix_fmt_bps_avi[] = {
> { AV_PIX_FMT_NONE, 0 },
> };
>
> -const PixelFormatTag avpriv_pix_fmt_bps_mov[] = {
> +static const PixelFormatTag pix_fmt_bps_mov[] = {
> { AV_PIX_FMT_PAL8, 1 },
> { AV_PIX_FMT_PAL8, 2 },
> { AV_PIX_FMT_PAL8, 4 },
> @@ -337,3 +337,33 @@ const PixelFormatTag avpriv_pix_fmt_bps_mov[] = {
> { AV_PIX_FMT_PAL8, 33 },
> { AV_PIX_FMT_NONE, 0 },
> };
> +
> +static enum AVPixelFormat find_pix_fmt(const PixelFormatTag *tags,
> + unsigned int fourcc)
> +{
> + while (tags->pix_fmt != AV_PIX_FMT_NONE) {
> + if (tags->fourcc == fourcc)
> + return tags->pix_fmt;
> + tags++;
> + }
> + return AV_PIX_FMT_NONE;
> +}
> +
> +enum AVPixelFormat avpriv_pix_fmt_find(enum PixelFormatTagLists list,
> + unsigned fourcc)
> +{
> + const PixelFormatTag *tags;
> +
> + switch (list) {
> + case PIX_FMT_LIST_RAW:
> + tags = raw_pix_fmt_tags;
> + break;
> + case PIX_FMT_LIST_AVI:
> + tags = pix_fmt_bps_avi;
> + break;
> + case PIX_FMT_LIST_MOV:
> + tags = pix_fmt_bps_mov;
> + break;
> + }
> + return find_pix_fmt(tags, fourcc);
> +}
> diff --git a/libavcodec/raw.h b/libavcodec/raw.h
> index 6a041690b1..9a4ddef8fc 100644
> --- a/libavcodec/raw.h
> +++ b/libavcodec/raw.h
> @@ -28,20 +28,21 @@
> #define AVCODEC_RAW_H
>
> #include "libavutil/pixfmt.h"
> -#include "internal.h"
>
> typedef struct PixelFormatTag {
> enum AVPixelFormat pix_fmt;
> unsigned int fourcc;
> } PixelFormatTag;
>
> -extern const PixelFormatTag ff_raw_pix_fmt_tags[]; // exposed through avpriv_get_raw_pix_fmt_tags()
> -
> const struct PixelFormatTag *avpriv_get_raw_pix_fmt_tags(void);
>
> -enum AVPixelFormat avpriv_find_pix_fmt(const PixelFormatTag *tags, unsigned int fourcc);
> +enum PixelFormatTagLists {
> + PIX_FMT_LIST_RAW,
> + PIX_FMT_LIST_AVI,
> + PIX_FMT_LIST_MOV,
> +};
>
> -extern av_export_avcodec const PixelFormatTag avpriv_pix_fmt_bps_avi[];
> -extern av_export_avcodec const PixelFormatTag avpriv_pix_fmt_bps_mov[];
> +enum AVPixelFormat avpriv_pix_fmt_find(enum PixelFormatTagLists list,
> + unsigned fourcc);
>
> #endif /* AVCODEC_RAW_H */
> diff --git a/libavcodec/rawdec.c b/libavcodec/rawdec.c
> index b22e36e984..9724cce13f 100644
> --- a/libavcodec/rawdec.c
> +++ b/libavcodec/rawdec.c
> @@ -76,15 +76,15 @@ static av_cold int raw_init_decoder(AVCodecContext *avctx)
>
> if ( avctx->codec_tag == MKTAG('r','a','w',' ')
> || avctx->codec_tag == MKTAG('N','O','1','6'))
> - avctx->pix_fmt = avpriv_find_pix_fmt(avpriv_pix_fmt_bps_mov,
> + avctx->pix_fmt = avpriv_pix_fmt_find(PIX_FMT_LIST_MOV,
> avctx->bits_per_coded_sample);
> else if (avctx->codec_tag == MKTAG('W', 'R', 'A', 'W'))
> - avctx->pix_fmt = avpriv_find_pix_fmt(avpriv_pix_fmt_bps_avi,
> + avctx->pix_fmt = avpriv_pix_fmt_find(PIX_FMT_LIST_AVI,
> avctx->bits_per_coded_sample);
> else if (avctx->codec_tag && (avctx->codec_tag & 0xFFFFFF) != MKTAG('B','I','T', 0))
> - avctx->pix_fmt = avpriv_find_pix_fmt(ff_raw_pix_fmt_tags, avctx->codec_tag);
> + avctx->pix_fmt = avpriv_pix_fmt_find(PIX_FMT_LIST_RAW, avctx->codec_tag);
> else if (avctx->pix_fmt == AV_PIX_FMT_NONE && avctx->bits_per_coded_sample)
> - avctx->pix_fmt = avpriv_find_pix_fmt(avpriv_pix_fmt_bps_avi,
> + avctx->pix_fmt = avpriv_pix_fmt_find(PIX_FMT_LIST_AVI,
> avctx->bits_per_coded_sample);
>
> desc = av_pix_fmt_desc_get(avctx->pix_fmt);
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index a91a54b0dc..4d236ff1cd 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -436,17 +436,6 @@ void ff_color_frame(AVFrame *frame, const int c[4])
> }
> }
>
> -enum AVPixelFormat avpriv_find_pix_fmt(const PixelFormatTag *tags,
> - unsigned int fourcc)
> -{
> - while (tags->pix_fmt >= 0) {
> - if (tags->fourcc == fourcc)
> - return tags->pix_fmt;
> - tags++;
> - }
> - return AV_PIX_FMT_NONE;
> -}
> -
> int avpriv_codec_get_cap_skip_frame_fill_param(const AVCodec *codec){
> return !!(codec->caps_internal & FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM);
> }
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index ef78781865..ec1501ef8e 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -51,7 +51,7 @@ static enum AVPixelFormat dshow_pixfmt(DWORD biCompression, WORD biBitCount)
> return AV_PIX_FMT_0RGB32;
> }
> }
> - return avpriv_find_pix_fmt(avpriv_get_raw_pix_fmt_tags(), biCompression); // all others
> + return avpriv_pix_fmt_find(PIX_FMT_LIST_RAW, biCompression); // all others
> }
>
> static int
> diff --git a/libavformat/avienc.c b/libavformat/avienc.c
> index 3fbde0be1e..be2493ce55 100644
> --- a/libavformat/avienc.c
> +++ b/libavformat/avienc.c
> @@ -448,7 +448,7 @@ static int avi_write_header(AVFormatContext *s)
> par->bits_per_coded_sample = 16;
> avist->pal_offset = avio_tell(pb) + 40;
> ff_put_bmp_header(pb, par, 0, 0, avi->flipped_raw_rgb);
> - pix_fmt = avpriv_find_pix_fmt(avpriv_pix_fmt_bps_avi,
> + pix_fmt = avpriv_pix_fmt_find(PIX_FMT_LIST_AVI,
> par->bits_per_coded_sample);
> if ( !par->codec_tag
> && par->codec_id == AV_CODEC_ID_RAWVIDEO
> diff --git a/libavformat/demux.c b/libavformat/demux.c
> index 745dc8687c..360abd6014 100644
> --- a/libavformat/demux.c
> +++ b/libavformat/demux.c
> @@ -2795,7 +2795,7 @@ int avformat_find_stream_info(AVFormatContext *ic, AVDictionary **options)
> if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
> if (avctx->codec_id == AV_CODEC_ID_RAWVIDEO && !avctx->codec_tag && !avctx->bits_per_coded_sample) {
> uint32_t tag= avcodec_pix_fmt_to_codec_tag(avctx->pix_fmt);
> - if (avpriv_find_pix_fmt(avpriv_get_raw_pix_fmt_tags(), tag) == avctx->pix_fmt)
> + if (avpriv_pix_fmt_find(PIX_FMT_LIST_RAW, tag) == avctx->pix_fmt)
> avctx->codec_tag= tag;
> }
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 0f912dd012..7216331fa1 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -1636,7 +1636,7 @@ static int mov_get_rawvideo_codec_tag(AVFormatContext *s, MOVTrack *track)
> }
> }
>
> - pix_fmt = avpriv_find_pix_fmt(avpriv_pix_fmt_bps_mov,
> + pix_fmt = avpriv_pix_fmt_find(PIX_FMT_LIST_MOV,
> track->par->bits_per_coded_sample);
> if (tag == MKTAG('r','a','w',' ') &&
> track->par->format != pix_fmt &&
>
Ping for this patchset. (I already applied the trivial first patch.)
- Andreas
More information about the ffmpeg-devel
mailing list