[FFmpeg-devel] [PATCH 05/30] Replace arrays of pointers to strings by arrays of strings
James Almer
jamrial at gmail.com
Thu Dec 31 02:26:11 EET 2020
On 12/30/2020 9:19 PM, Andreas Rheinhardt wrote:
> 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?
Yes, but it sounds like an extreme micro-optimization to still depends
on the compiler being smart enough.
Either way is fine by me in any case.
>
>>> 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".
>
> _______________________________________________
> 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