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

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

2015-12-02 Thread Ganesh Ajjanagadde

On Tue, Dec 1, 2015 at 8:38 PM, James Almer  wrote:
> On 12/1/2015 10:35 PM, Ganesh Ajjanagadde wrote:
>> On Tue, Dec 1, 2015 at 8:08 PM, James Almer  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 
 ---
  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@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2015-12-01 Thread James Almer

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 
> ---
>  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.

> +#endif
>  
>  
>  /**
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2015-12-01 Thread Ganesh Ajjanagadde

On Tue, Dec 1, 2015 at 8:08 PM, James Almer  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 
>> ---
>>  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?

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

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

2015-12-01 Thread Ganesh Ajjanagadde

On Tue, Dec 1, 2015 at 8:38 PM, James Almer  wrote:
> On 12/1/2015 10:35 PM, Ganesh Ajjanagadde wrote:
>> On Tue, Dec 1, 2015 at 8:08 PM, James Almer  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 
 ---
  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.

[...]
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2015-12-01 Thread Ganesh Ajjanagadde

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 
---
 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))
+#endif
 
 
 /**
-- 
2.6.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2015-12-01 Thread James Almer

On 12/1/2015 10:35 PM, Ganesh Ajjanagadde wrote:
> On Tue, Dec 1, 2015 at 8:08 PM, James Almer  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 
>>> ---
>>>  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.

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

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

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

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

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

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

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

6 matches

Site Navigation

Mail list logo

Footer information