Alexander Strasser:
> On 2020-07-19 23:38 +0200, Andreas Rheinhardt wrote:
>> Alexander Strasser:
> [...]
>>>
>>> On 2020-07-17 13:14 +0200, Andreas Rheinhardt wrote:
>>>> Andreas Rheinhardt:
>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
>>>>> ---
>>>>> libavformat/au.c | 13 ++++++-------
>>>>> 1 file changed, 6 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/libavformat/au.c b/libavformat/au.c
>>>>> index f92863e400..b419c9ed95 100644
>>>>> --- a/libavformat/au.c
>>>>> +++ b/libavformat/au.c
>>>>> @@ -68,13 +68,13 @@ static int au_probe(const AVProbeData *p)
>>>>>
>>>>> static int au_read_annotation(AVFormatContext *s, int size)
>>>>> {
>>>>> - static const char * keys[] = {
>>>>> + static const char keys[][7] = {
>>>>> "title",
>>>>> "artist",
>>>>> "album",
>>>>> "track",
>>>>> "genre",
>>>>> - NULL };
>>>>> + };
>>>
>>> Sorry, I couldn't test myself but wouldn't just
>>>
>>> static const char * keys[] = {
>>> "title",
>>> "artist",
>>> "album",
>>> "track",
>>> "genre",
>>> };
>>>
>>> have been the better choice with the same benefits?
>>>
>>> I'm not sure without looking up the C standard and writing some
>>> little test programs. From my guts I would say it's equivalent,
>>> but the syntax is more convenient this way.
>>>
>>> That's also another issue with the commit message, since I don't
>>> think it is true that in your version strings are stored in the
>>> array. No offense intended and I sure might be mistaken.
>>>
>> But it is true.
>
> Yes, you're right. I read the array dimension the wrong way around :(
>
> Sorry for the noise.
>
>
>> The strings are stored directly in the array, so that
>> each array takes up 5 * 7 bytes. Before the change, the array itself
>> took up 5 * sizeof(char *). And on top of that comes the space for the
>> strings itself.
>>
>> Storing the strings itself in the array instead of pointers to the
>> strings saves the space of the pointers, but it forces the shortest
>> string to the size of the longest string. Therefore it is worthwhile if
>> the average size differs from the longest size by less than sizeof(char
>> *); there is furthermore one exception to this rule: If one stores
>> pointers, different pointers may point to the same string (or to a part
>> of a larger string). In the case here, the strings itself are smaller
>> than sizeof(char *) on 64bit systems, so this alone shows that one saves
>> space.
>
> So it's about 40 bytes on systems with 64 addresses, right?
>
6 pointers amount to 48 bytes, but one also has to waste 4 bytes to make
the shorter strings as long as the longest.
>
>> Also notice that there are two more differences:
>> a) The earlier version consisted of an array of nonconst pointers to
>> const strings. Making the array entries const has been forgotten (it
>> would go like this: "static const char *const keys[]"). Given that
>> arrays are automatically nonmodifiable, this problem doesn't exist.
>> (Given that these arrays are static and given that the compiler can see
>> that they are not modified means that the compiler could infer that they
>> are const and put them in the appropriate section for const data.)
>> b) The actual access to the string is different: If the array contains
>> pointer to strings, then one has to read the array entry (containing the
>> pointer to the string) and pass it to av_strcasecmp() etc. If the array
>> contains the strings, then one just has to get the address of the array
>> entry (which coincides with the address of the string itself) and pass
>> it to av_strcasecmp() etc..
>
> I agree the new version is technically superior.
>
> It's also more rigid, but that should probably not matter much in
> this specific case.
>
Indeed, it is usually not suitable when one has too many strings.
> Still I think the savings are marginal, so I guess you did them
> while looking into the code for fixing the things later in this
> patch set.
>
Yes.
- Andreas
_______________________________________________
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".
