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

[Ganesh Ajjanagadde]

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.

pushed with an additional header include version.h needed to work
correctly, thanks.

>
>>
>>>
>>>> +#endif
>>>>
>>>>
>>>>  /**
>>>>
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel