[FFmpeg-devel] [PATCHv2] avutil/crc: avoid needless space wastage of hardcoded crc table

Ganesh Ajjanagadde gajjanag at mit.edu
Wed Dec 2 03:00:55 CET 2015


On Tue, Dec 1, 2015 at 8:38 PM, James Almer <jamrial at gmail.com> wrote:
> On 12/1/2015 10:35 PM, Ganesh Ajjanagadde wrote:
>> On Tue, Dec 1, 2015 at 8:08 PM, James Almer <jamrial at gmail.com> wrote:
>>> On 12/1/2015 9:53 PM, Ganesh Ajjanagadde wrote:
>>>> There was no reason AFAIK for making AV_CRC_24_IEEE 12. This simply
>>>> resulted in wasted space under --enable-hardcoded-tables:
>>>> dynamic: 1318672 libavutil/libavutil.so.55
>>>> old    : 1330680 libavutil/libavutil.so.55
>>>> new    : 1326488 libavutil/libavutil.so.55
>>>>
>>>> Minor version number is bumped, with ifdefry due to API breakage.
>>>>
>>>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>>>> ---
>>>>  libavutil/crc.h     | 4 ++++
>>>>  libavutil/version.h | 5 ++++-
>>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavutil/crc.h b/libavutil/crc.h
>>>> index e86bf1d..9af4421 100644
>>>> --- a/libavutil/crc.h
>>>> +++ b/libavutil/crc.h
>>>> @@ -40,7 +40,11 @@ typedef enum {
>>>>      AV_CRC_32_IEEE,
>>>>      AV_CRC_32_IEEE_LE,  /*< reversed bitorder version of AV_CRC_32_IEEE */
>>>>      AV_CRC_16_ANSI_LE,  /*< reversed bitorder version of AV_CRC_16_ANSI */
>>>> +#if FF_API_CRC_BIG_TABLE
>>>>      AV_CRC_24_IEEE = 12,
>>>> +#else
>>>> +    AV_CRC_24_IEEE,
>>>> +#endif /* FF_API_CRC_BIG_TABLE */
>>>>      AV_CRC_MAX,         /*< Not part of public API! Do not use outside libavutil. */
>>>>  }AVCRCId;
>>>>
>>>> diff --git a/libavutil/version.h b/libavutil/version.h
>>>> index e0ddfd2..67e778a 100644
>>>> --- a/libavutil/version.h
>>>> +++ b/libavutil/version.h
>>>> @@ -56,7 +56,7 @@
>>>>   */
>>>>
>>>>  #define LIBAVUTIL_VERSION_MAJOR  55
>>>> -#define LIBAVUTIL_VERSION_MINOR   9
>>>> +#define LIBAVUTIL_VERSION_MINOR  10
>>>>  #define LIBAVUTIL_VERSION_MICRO 100
>>>>
>>>>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>>>> @@ -108,6 +108,9 @@
>>>>  #ifndef FF_API_ERROR_FRAME
>>>>  #define FF_API_ERROR_FRAME              (LIBAVUTIL_VERSION_MAJOR < 56)
>>>>  #endif
>>>> +#ifndef FF_API_CRC_BIG_TABLE
>>>> +#define FF_API_CRC_BIG_TABLE            (LIBAVUTIL_VERSION_INT < AV_VERSION_INT(55, 10, 100))
>>>
>>> This is wrong. With this check, FF_API_CRC_BIG_TABLE is false. The point
>>> of these FF_API defines is to make sure they are true until the next major
>>> bump. That's why you use LIBAVUTIL_VERSION_MAJOR < 56, like it's being
>>> done for the rest.
>>
>> Finally getting a sense for these things, thanks. Barring that, does
>> it look fine?
>
> Yes. You can also skip bumping minor. You're scheduling a change for the next
> major bump and not making any changes with immediate effects, so a minor bump
> is not necessary.

Would like to ask for comments regarding always hard or always soft
tables. Clearly, with this change (to be pushed soon), the space cost
of hardcoded tables is reduced once we do a major bump, and the
overhead of --enable-hardcoded-tables here is ~8 kB, down from ~ 12
kB.

I personally have a very slight preference for runtime tables here
(the default configure), since the runtime generation code anyway
needs to be present for nonstandard crc's but am definitely ok with
hardcoded as well. In any case, as the size is small, we should be
able to come to a decision here, and not continue to be on the fence
for this one unlike e.g mpegaudio_tablegen.

[...]


More information about the ffmpeg-devel mailing list