Re: [FFmpeg-devel] [PATCH 02/11] avcodec/raw: Reduce number of avpriv symbols

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-12-15 13:35:32)
>> libavcodec currently exports four avpriv symbols that deal with
>> PixelFormatTags: avpriv_get_raw_pix_fmt_tags, avpriv_find_pix_fmt,
>> avpriv_pix_fmt_bps_avi and avpriv_pix_fmt_bps_mov. The latter two are
>> lists of PixelFormatTags, the former returns such a list and the second
>> searches a list for a pixel format that matches a given fourcc; only
>> one of the aforementioned three lists is ever searched.
>>
>> Yet for avpriv_pix_fmt_bps_avi, avpriv_pix_fmt_bps_mov and
>> avpriv_find_pix_fmt the overhead of exporting these functions actually
>> exceeds the size of said objects (at least for ELF; the following numbers
>> are for x64 Ubuntu 20.10):
>> The code size of avpriv_find_pix_fmt is small (GCC 10.2 37B, Clang 11 41B),
>> yet exporting it adds a 20B string for the name alone to the exporting
>> as well as to each importing library; there is more: Four bytes in the
>> exporting libraries .gnu.hash; two bytes each for the exporting as well
>> as each importing libraries .gnu.version; 24B in the exporting as well
>> as each importing libraries .dynsym; 16B+24B for an entry in .plt as
>> well as the accompanying relocation entry in .rela.plt for each
>> importing library.
>>
>> The overhead for the lists is similar: The strings are 23B and the
>> .plt+.rela.plt pair is replaced by 8B+24B for an entry in .got and
>> a relocation entry in .rela.dyn. These lists have a size of 80 resp.
>> 72 bytes.
>>
>> Yet for ff_raw_pix_fmt_tags, exporting it is advantageous compared to
>> duplicating it into libavformat and potentially libavdevice. Therefore
>> this commit replaces all library uses of the four symbols with a single
>> function that is exported for shared builds. It has an enum parameter
>> to choose the desired list besides the parameter for the fourcc. New
>> lists can be supported with new enum values.
>>
>> Unfortunately, avpriv_get_raw_pix_fmt_tags could not be removed, as the
>> fourcc2pixfmt tool uses the table of raw pix fmts. No other user of this
>> function remains.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> 1. It would make sense to deduplicate avpriv_pix_fmt_bps_(mov|avi)
>> size-wise if it is preferred to keep the lists accessible to users.
>> 2. One could handle the fourcc2pixfmt case in avpriv_pix_fmt_find(),
>> too, if one added another parameter (say count). In this case
>> avpriv_pix_fmt_find() will return the first pixfmt with the desired
>> fourcc if count is zero, the second one is count is 1 etc. (and
>> AV_PIX_FMT_NONE in case there is none any more). This would also make
>> this function more future-proof.
> 
> Would it not be simpler to add a second function returning a pointer to
> the full table?
> 

There is already a function to return the full table:
avpriv_get_raw_pix_fmt_tags. It is what fourcc2pixfmt uses. The goal of
the above considerations is to reduce the number of exported symbols,
namely avpriv_get_raw_pix_fmt_tags.

> Also, I would prefer making this completely public rather than avpriv.
> 

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".