| Message ID | AM7PR03MB6660DF244AE3C4B8392D76848F769@AM7PR03MB6660.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,01/11] avcodec/Makefile: Remove superfluous avformat->DNXHD dependencies | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
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@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
Quoting Andreas Rheinhardt (2021-12-15 13:35:32) > 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@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. Would it not be simpler to add a second function returning a pointer to the full table? Also, I would prefer making this completely public rather than avpriv.
Anton Khirnov: > Quoting Andreas Rheinhardt (2021-12-15 13:35:32) >> 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@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. > > Would it not be simpler to add a second function returning a pointer to > the full table? > There is already a function to return the full table: avpriv_get_raw_pix_fmt_tags. It is what fourcc2pixfmt uses. The goal of the above considerations is to reduce the number of exported symbols, namely avpriv_get_raw_pix_fmt_tags. > Also, I would prefer making this completely public rather than avpriv. >
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@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 && > Will apply this patchset tomorrow unless there are objections. - Andreas
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 &&
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@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(-)