[FFmpeg-devel] [PATCH 05/30] Replace arrays of pointers to strings by arrays of strings
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Thu Dec 31 02:19:06 EET 2020
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.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?
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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list