Re: [FFmpeg-devel] [PATCH 8/8] avcodec/codec_internal: Include codec_tags only when they are needed

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

Michael Niedermayer:
> On Fri, Mar 18, 2022 at 11:52:54AM +0100, Andreas Rheinhardt wrote:
>> They are only needed for the fuzzer, so check for CONFIG_OSSFUZZ.
>> This decreases sizeof(FFCodec), which is important given that
>> FFCodecs reside in .data.rel.ro in case of ELF with
>> position-independent code which is always loaded and can't be shared
>> between processes.
>>
> 
> They are currently only used by the fuzzer, if there is no other
> use, iam not sure. But i agree shareable memory would be better for them
> 

GCC by default aligns big enough objects to 32* (so while sizeof(codec)
need not be a multiple of said alignment, the actual object is still
padded to said alignment) and it was aligned to 32 before the addition
of ch_layouts (for x64), so adding it increased the size by 32 (and
resulted in over 21KiB size increase here on a really
not-all-encompassing build). Optimizing codec_tags away would reverse this.

> 
> [...]
>> diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
>> index 288aa63313..77f4bb8dd8 100644
>> --- a/tools/target_dec_fuzzer.c
>> +++ b/tools/target_dec_fuzzer.c
>> @@ -279,12 +279,14 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
>>          ctx->sample_rate                        = bytestream2_get_le32(&gbc) & 0x7FFFFFFF;
>>          ctx->ch_layout.nb_channels              = (unsigned)bytestream2_get_le32(&gbc) % FF_SANE_NB_CHANNELS;
>>          ctx->block_align                        = bytestream2_get_le32(&gbc) & 0x7FFFFFFF;
>> +#if CONFIG_OSSFUZZ
>>          ctx->codec_tag                          = bytestream2_get_le32(&gbc);
>>          if (c->codec_tags) {
>>              int n;
>>              for (n = 0; c->codec_tags[n] != FF_CODEC_TAGS_END; n++);
>>              ctx->codec_tag = c->codec_tags[ctx->codec_tag % n];
>>          }
>> +#endif
>>          keyframes                               = bytestream2_get_le64(&gbc);
>>          request_channel_layout                  = bytestream2_get_le64(&gbc);
>>  
> 
> how does the fuzzer work without the fuzzer ?
> 

You could just not use --enable-ossfuzz, but nevertheless want to "make
tools/target_dec_foo_fuzzer" (with custom fuzzer CFLAGS/LDFLAGS). But it
is not really the intended usecase, so I made sure it compiles, but
nothing more.

- Andreas

*: The ELF-x64-ABI only requires 16, but it uses 32 for compatibility
with ancient versions of GCC which required 32. One can use
-malign-data=abi to make it only use 16.
_______________________________________________
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".