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

James Almer jamrial at gmail.com
Mon Nov 30 04:07:24 CET 2015


On 11/30/2015 12:02 AM, Ganesh Ajjanagadde wrote:
> On Sun, Nov 29, 2015 at 9:58 PM, James Almer <jamrial at gmail.com> wrote:
>> On 11/29/2015 11:45 PM, Ronald S. Bultje wrote:
>>> Hi,
>>>
>>> On Sun, Nov 29, 2015 at 9:41 PM, Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>>> 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.
>>>>
>>>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>>>> ---
>>>>  libavutil/crc.h     | 2 +-
>>>>  libavutil/version.h | 2 +-
>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/libavutil/crc.h b/libavutil/crc.h
>>>> index e86bf1d..61592df 100644
>>>> --- a/libavutil/crc.h
>>>> +++ b/libavutil/crc.h
>>>> @@ -40,7 +40,7 @@ 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 */
>>>> -    AV_CRC_24_IEEE = 12,
>>>> +    AV_CRC_24_IEEE,
>>>>      AV_CRC_MAX,         /*< Not part of public API! Do not use outside
>>>> libavutil. */
>>>>  }AVCRCId;
>>>
>>>
>>> I support the idea, but this breaks ABI. You need to do this under a
>>> version bump, see libavutil/version.h for templates.
>>
>> One could argue no releases have been made since the last major bump, so breakages
>> like this are unlikely to affect anyone. But then again, it's been months since said
>> major bump.
>> What's been chosen in previous bumps as the drawing line for ABI breakages? Some
>> arbitrary amount of time after the last bump (like the month mentioned in APIChanges),
>> or first release post bump?
> 
> Can one include version.h in crc.h, test if version >= , version < ,
> etc using the preprocessor, and accordingly define the enum?

Yes. As Ronald said, check version.h for templates about how to schedule ABI changes and
API deprecations. See the FF_API defines and grep the tree to see how they are used.

That aside, your patch may be able to go in without having to litter the code with
unnecessary FF_API or similar ifdeffery depending on what others say about my question
above.

> 
>>
>>>
>>> Ronald
>>> _______________________________________________
>>> 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
> 



More information about the ffmpeg-devel mailing list