James Almer: > 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.rheinha...@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?
sizeof("IDR") would be better, I think. I can do so. > 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? > It could be three, but don't processors have special instructions for ptr + i * offset for certain powers of two like 4? >> 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" >> }; >> > > _______________________________________________ > 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". _______________________________________________ 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".