Re: [FFmpeg-devel] [PATCH 04/11] avcodec/ac3tab: Unavpriv ac3_channel_layout_tab

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

Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-12-15 13:35:34)
>> It is small (16 B) and therefore the overhead of exporting it more
>> than outweighs the size savings from not having duplicated symbols:
>> When the symbol is no longer avpriv, one saves twice the size of
>> the string containing the symbols name (2x30 byte), two entries
>> in .dynsym (24 bytes each on x64), one entry in the importing libraries
>> .got and .rela.dyn (8 + 24 bytes on x64) and two entries for the
>> symbol version (2 bytes each) and one hash value in the exporting
>> library (4 bytes).
>> (The exact numbers are of course different for other platforms
>> (e.g. when using dlls), but given that the strings saved alone
>> more than outweigh the array size it can be presumed that this
>> is beneficial for all platforms.)
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  libavcodec/Makefile                  | 12 +++++---
>>  libavcodec/ac3_channel_layout_tab.c  | 22 +++++++++++++++
>>  libavcodec/ac3_channel_layout_tab.h  | 41 ++++++++++++++++++++++++++++
>>  libavcodec/ac3_parser.c              |  2 +-
>>  libavcodec/ac3dec.c                  |  4 +--
>>  libavcodec/ac3enc.h                  |  1 +
>>  libavcodec/ac3tab.c                  | 14 ----------
>>  libavcodec/ac3tab.h                  |  4 +--
>>  libavcodec/eac3_data.c               |  2 ++
>>  libavformat/Makefile                 |  2 ++
>>  libavformat/ac3_channel_layout_tab.c | 22 +++++++++++++++
>>  libavformat/hls_sample_encryption.c  |  4 ++-
>>  libavformat/mov.c                    |  4 +--
>>  13 files changed, 107 insertions(+), 27 deletions(-)
>>  create mode 100644 libavcodec/ac3_channel_layout_tab.c
>>  create mode 100644 libavcodec/ac3_channel_layout_tab.h
>>  create mode 100644 libavformat/ac3_channel_layout_tab.c
>>
>> diff --git a/libavcodec/ac3enc.h b/libavcodec/ac3enc.h
>> index ec9ead8a4e..39a41fe0b0 100644
>> --- a/libavcodec/ac3enc.h
>> +++ b/libavcodec/ac3enc.h
>> @@ -35,6 +35,7 @@
>>  #include "ac3dsp.h"
>>  #include "avcodec.h"
>>  #include "fft.h"
>> +#include "internal.h"
> 
> ?

Before this commit, ac3tab.h included internal.h for av_export_avcodec
and ac3enc.h indirectly includes ac3tab.h via ac3.h. Yet ac3enc.h
provides ff_ac3_enc_defaults of type AVCodecDefaults. This type is
defined in internal.h, so ac3enc.h should properly include internal.h.

> 
>> diff --git a/libavcodec/eac3_data.c b/libavcodec/eac3_data.c
>> index b159e1682f..2ef0e2053c 100644
>> --- a/libavcodec/eac3_data.c
>> +++ b/libavcodec/eac3_data.c
>> @@ -24,6 +24,8 @@
>>   * Tables taken directly from the E-AC-3 spec.
>>   */
>>  
>> +#include <stddef.h>
> 
> ??

This file has a NULL which it got implicitly via internal.h included by
ac3tab.h (yes, really). This has been changed, so this header needs to
be added.

> 
>> diff --git a/libavformat/hls_sample_encryption.c b/libavformat/hls_sample_encryption.c
>> index 38795c7fb0..3dbaff717e 100644
>> --- a/libavformat/hls_sample_encryption.c
>> +++ b/libavformat/hls_sample_encryption.c
>> @@ -26,6 +26,8 @@
>>   * https://developer.apple.com/library/ios/documentation/AudioVideo/Conceptual/HLS_Sample_Encryption
>>   */
>>  
>> +#include "libavutil/channel_layout.h"

This header (which uses e.g. AV_CH_LOW_FREQUENCY and
av_get_channel_layout_nb_channels()) is currently implicitly included
via libavcodec/internal.h included by ac3tab.h.

> 
> ???
> 

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