[FFmpeg-devel] [PATCH 05/30] Replace arrays of pointers to strings by arrays of strings
James Almer
jamrial at gmail.com
Thu Dec 31 01:51:16 EET 2020
On 12/30/2020 8:31 PM, Andreas Rheinhardt wrote:
> When the difference of the longest size and the average size of
> collection of strings is smaller than the size of a pointer, it makes
> sense to store the strings directly in an array instead of using an
> array of pointers to strings (unless doing so precludes deduplicating
> strings); doing so also avoids relocations. This requirement is
> fulfilled for several arrays of pointers to strings that are changed in
> this commit.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> 1. The changes to ccaption_dec (which provide the largest savings of
> this patchset) require \u to yield UTF-8; I hope this is ok. Notice that
> if it were otherwise we would already output non UTF-8.
> 2. Whether it is worthwhile to replace arrays of pointers to strings by
> arrays of strings depends of course upon the size of pointers. Do I need
> to take 32bit systems into account for further patches like this here?
> 3. On 64bit Elf systems every one of these relocations takes 24bytes; it
> is less on Windows. Here is more about this:
> https://chromium.googlesource.com/chromium/src/+/master/docs/native_relocations.md
>
> libavcodec/ccaption_dec.c | 4 ++--
> libavcodec/vaapi_encode.c | 2 +-
> libavfilter/af_hdcd.c | 2 +-
> libavfilter/f_perms.c | 2 +-
> libavformat/iff.c | 4 ++--
> libavformat/matroskadec.c | 2 +-
> libavformat/sdp.c | 2 +-
> libavutil/parseutils.c | 2 +-
> 8 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
> index a208e19b95..c864416241 100644
> --- a/libavcodec/ccaption_dec.c
> +++ b/libavcodec/ccaption_dec.c
> @@ -66,7 +66,7 @@ enum cc_charset {
> CCSET_EXTENDED_PORTUGUESE_GERMAN_DANISH,
> };
>
> -static const char *charset_overrides[4][128] =
> +static const char charset_overrides[4][128][4] =
> {
> [CCSET_BASIC_AMERICAN] = {
> [0x27] = "\u2019",
> @@ -575,7 +575,7 @@ static int capture_screen(CCaptionSubContext *ctx)
> prev_color = color[j];
> prev_bg_color = bg[j];
> override = charset_overrides[(int)charset[j]][(int)row[j]];
> - if (override) {
> + if (override[0]) {
> av_bprintf(&ctx->buffer[bidx], "%s%s%s%s%s", e_tag, s_tag, c_tag, b_tag, override);
> seen_char = 1;
> } else if (row[j] == ' ' && !seen_char) {
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 518e5b2c00..a47facc46d 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -33,7 +33,7 @@ const AVCodecHWConfigInternal *const ff_vaapi_encode_hw_configs[] = {
> NULL,
> };
>
> -static const char * const picture_type_name[] = { "IDR", "I", "P", "B" };
> +static const char picture_type_name[][4] = { "IDR", "I", "P", "B" };
This and every other case are not obvious at first glance. Could you add
a comment next to the size like /* strlen("IDR") + 1 */ or similar to
all of them?
If someone were to add new entries that are longer than the current
longest one, it would break. The iff ones especially look like could
perhaps be extended (Whereas the parseutils array is the most likely to
stay unchanged for obvious reasons).
>
> static int vaapi_encode_make_packed_header(AVCodecContext *avctx,
> VAAPIEncodePicture *pic,
> diff --git a/libavfilter/af_hdcd.c b/libavfilter/af_hdcd.c
> index 251d03229a..7000ea81de 100644
> --- a/libavfilter/af_hdcd.c
> +++ b/libavfilter/af_hdcd.c
> @@ -900,7 +900,7 @@ typedef enum {
> HDCD_PVER_MIX = 3, /**< Packets of type A and B discovered, most likely an encoding error */
> } hdcd_pf;
>
> -static const char * const pf_str[] = {
> +static const char pf_str[][4] = {
> "?", "A", "B", "A+B"
> };
>
> diff --git a/libavfilter/f_perms.c b/libavfilter/f_perms.c
> index d984e5b150..ad52a4ed6e 100644
> --- a/libavfilter/f_perms.c
> +++ b/libavfilter/f_perms.c
> @@ -72,7 +72,7 @@ static av_cold int init(AVFilterContext *ctx)
> }
>
> enum perm { RO, RW };
> -static const char * const perm_str[2] = { "RO", "RW" };
> +static const char perm_str[2][4] = { "RO", "RW" };
Shouldn't this one be 3?
>
> static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
> {
> diff --git a/libavformat/iff.c b/libavformat/iff.c
> index 2dba121f6f..53abd6d168 100644
> --- a/libavformat/iff.c
> +++ b/libavformat/iff.c
> @@ -199,13 +199,13 @@ static const uint64_t dsd_loudspeaker_config[] = {
> AV_CH_LAYOUT_5POINT0, AV_CH_LAYOUT_5POINT1,
> };
>
> -static const char * dsd_source_comment[] = {
> +static const char dsd_source_comment[][24] = {
> "dsd_source_comment",
> "analogue_source_comment",
> "pcm_source_comment",
> };
>
> -static const char * dsd_history_comment[] = {
> +static const char dsd_history_comment[][17] = {
> "general_remark",
> "operator_name",
> "creating_machine",
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 374831baa3..300ccae0ad 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1946,7 +1946,7 @@ static void matroska_parse_cues(MatroskaDemuxContext *matroska) {
>
> static int matroska_aac_profile(char *codec_id)
> {
> - static const char *const aac_profiles[] = { "MAIN", "LC", "SSR" };
> + static const char aac_profiles[][5] = { "MAIN", "LC", "SSR" };
> int profile;
>
> for (profile = 0; profile < FF_ARRAY_ELEMS(aac_profiles); profile++)
> diff --git a/libavformat/sdp.c b/libavformat/sdp.c
> index 95f3fbb876..5a0e1695c9 100644
> --- a/libavformat/sdp.c
> +++ b/libavformat/sdp.c
> @@ -230,7 +230,7 @@ static char *extradata2psets_hevc(AVCodecParameters *par)
> int extradata_size = par->extradata_size;
> uint8_t *tmpbuf = NULL;
> int ps_pos[3] = { 0 };
> - static const char * const ps_names[3] = { "vps", "sps", "pps" };
> + static const char ps_names[3][4] = { "vps", "sps", "pps" };
> int num_arrays, num_nalus;
> int pos, i, j;
>
> diff --git a/libavutil/parseutils.c b/libavutil/parseutils.c
> index 167e822648..fffe338f56 100644
> --- a/libavutil/parseutils.c
> +++ b/libavutil/parseutils.c
> @@ -140,7 +140,7 @@ static const VideoRateAbbr video_rate_abbrs[]= {
> { "ntsc-film", { 24000, 1001 } },
> };
>
> -static const char *months[12] = {
> +static const char months[12][10] = {
> "january", "february", "march", "april", "may", "june", "july", "august",
> "september", "october", "november", "december"
> };
>
More information about the ffmpeg-devel
mailing list