[FFmpeg-devel] [PATCH 8/8] avcodec/codec_internal: Include codec_tags only when they are needed
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. Signed-off-by: Andreas Rheinhardt --- libavcodec/bitpacked_dec.c | 5 + libavcodec/codec_internal.h | 10 ++ libavcodec/hapdec.c | 13 + tools/target_dec_fuzzer.c | 2 ++ 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/libavcodec/bitpacked_dec.c b/libavcodec/bitpacked_dec.c index 419550dfe0..b62d88fa8f 100644 --- a/libavcodec/bitpacked_dec.c +++ b/libavcodec/bitpacked_dec.c @@ -151,9 +151,6 @@ const FFCodec ff_bitpacked_decoder = { .init = bitpacked_init_decoder, .decode = bitpacked_decode, .p.capabilities = AV_CODEC_CAP_FRAME_THREADS, -.codec_tags = (const uint32_t []){ -MKTAG('U', 'Y', 'V', 'Y'), -FF_CODEC_TAGS_END, -}, .caps_internal = FF_CODEC_CAP_INIT_THREADSAFE, +FF_CODEC_TAGS(MKTAG('U', 'Y', 'V', 'Y')) }; diff --git a/libavcodec/codec_internal.h b/libavcodec/codec_internal.h index 596cdbebd2..b6b5b05b44 100644 --- a/libavcodec/codec_internal.h +++ b/libavcodec/codec_internal.h @@ -21,6 +21,7 @@ #include +#include "config.h" #include "libavutil/attributes.h" #include "codec.h" @@ -74,10 +75,16 @@ */ #define FF_CODEC_CAP_SETS_FRAME_PROPS (1 << 8) +#if CONFIG_OSSFUZZ /** * FFCodec.codec_tags termination value */ #define FF_CODEC_TAGS_END -1 +#define FF_CODEC_TAGS(...) \ +.codec_tags = (const uint32_t[]){ __VA_ARGS__, FF_CODEC_TAGS_END }, +#else +#define FF_CODEC_TAGS(...) +#endif typedef struct FFCodecDefault { const char *key; @@ -196,10 +203,13 @@ typedef struct FFCodec { */ const struct AVCodecHWConfigInternal *const *hw_configs; +#if CONFIG_OSSFUZZ /** * List of supported codec_tags, terminated by FF_CODEC_TAGS_END. + * Should be defined with the FF_CODEC_TAGS() macro. */ const uint32_t *codec_tags; +#endif } FFCodec; static av_always_inline const FFCodec *ffcodec(const AVCodec *codec) diff --git a/libavcodec/hapdec.c b/libavcodec/hapdec.c index 4a7ac15a8e..72f922bc5b 100644 --- a/libavcodec/hapdec.c +++ b/libavcodec/hapdec.c @@ -486,12 +486,9 @@ const FFCodec ff_hap_decoder = { AV_CODEC_CAP_DR1, .caps_internal = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP, -.codec_tags = (const uint32_t []){ -MKTAG('H','a','p','1'), -MKTAG('H','a','p','5'), -MKTAG('H','a','p','Y'), -MKTAG('H','a','p','A'), -MKTAG('H','a','p','M'), -FF_CODEC_TAGS_END, -}, +FF_CODEC_TAGS(MKTAG('H','a','p','1'), + MKTAG('H','a','p','5'), + MKTAG('H','a','p','Y'), + MKTAG('H','a','p','A'), + MKTAG('H','a','p','M')) }; 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) & 0x7FFF; ctx->ch_layout.nb_channels = (unsigned)bytestream2_get_le32(&gbc) % FF_SANE_NB_CHANNELS; ctx->block_align= bytestream2_get_le32(&gbc) & 0x7FFF; +#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); -- 2.32.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 8/8] avcodec/codec_internal: Include codec_tags only when they are needed
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 [...] > 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) > & 0x7FFF; > ctx->ch_layout.nb_channels = > (unsigned)bytestream2_get_le32(&gbc) % FF_SANE_NB_CHANNELS; > ctx->block_align= bytestream2_get_le32(&gbc) > & 0x7FFF; > +#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 ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "You are 36 times more likely to die in a bathtub than at the hands of a terrorist. Also, you are 2.5 times more likely to become a president and 2 times more likely to become an astronaut, than to die in a terrorist attack." -- Thoughty2 signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 8/8] avcodec/codec_internal: Include codec_tags only when they are needed
On Fri, Mar 18, 2022 at 02:13:16PM +0100, Michael Niedermayer wrote: > 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 > > > [...] > > 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) & 0x7FFF; > > ctx->ch_layout.nb_channels = > > (unsigned)bytestream2_get_le32(&gbc) % FF_SANE_NB_CHANNELS; > > ctx->block_align= > > bytestream2_get_le32(&gbc) & 0x7FFF; > > +#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 ? or is the idea to build test this even without oss-fuzz somehow? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 8/8] avcodec/codec_internal: Include codec_tags only when they are needed
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) & 0x7FFF; >> ctx->ch_layout.nb_channels = >> (unsigned)bytestream2_get_le32(&gbc) % FF_SANE_NB_CHANNELS; >> ctx->block_align= >> bytestream2_get_le32(&gbc) & 0x7FFF; >> +#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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 8/8] avcodec/codec_internal: Include codec_tags only when they are needed
Michael Niedermayer: > On Fri, Mar 18, 2022 at 02:13:16PM +0100, Michael Niedermayer wrote: >> 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 >> >> >> [...] >>> 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) & 0x7FFF; >>> ctx->ch_layout.nb_channels = >>> (unsigned)bytestream2_get_le32(&gbc) % FF_SANE_NB_CHANNELS; >>> ctx->block_align= >>> bytestream2_get_le32(&gbc) & 0x7FFF; >>> +#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 ? > > or is the idea to build test this even without oss-fuzz somehow? > Yes, this #if CONFIG_OSSFUZZ is just intended to make it still compile in this case. - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 8/8] avcodec/codec_internal: Include codec_tags only when they are needed
Andreas Rheinhardt: > 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. > > Signed-off-by: Andreas Rheinhardt > --- > libavcodec/bitpacked_dec.c | 5 + > libavcodec/codec_internal.h | 10 ++ > libavcodec/hapdec.c | 13 + > tools/target_dec_fuzzer.c | 2 ++ > 4 files changed, 18 insertions(+), 12 deletions(-) > > diff --git a/libavcodec/bitpacked_dec.c b/libavcodec/bitpacked_dec.c > index 419550dfe0..b62d88fa8f 100644 > --- a/libavcodec/bitpacked_dec.c > +++ b/libavcodec/bitpacked_dec.c > @@ -151,9 +151,6 @@ const FFCodec ff_bitpacked_decoder = { > .init = bitpacked_init_decoder, > .decode = bitpacked_decode, > .p.capabilities = AV_CODEC_CAP_FRAME_THREADS, > -.codec_tags = (const uint32_t []){ > -MKTAG('U', 'Y', 'V', 'Y'), > -FF_CODEC_TAGS_END, > -}, > .caps_internal = FF_CODEC_CAP_INIT_THREADSAFE, > +FF_CODEC_TAGS(MKTAG('U', 'Y', 'V', 'Y')) > }; > diff --git a/libavcodec/codec_internal.h b/libavcodec/codec_internal.h > index 596cdbebd2..b6b5b05b44 100644 > --- a/libavcodec/codec_internal.h > +++ b/libavcodec/codec_internal.h > @@ -21,6 +21,7 @@ > > #include > > +#include "config.h" > #include "libavutil/attributes.h" > #include "codec.h" > > @@ -74,10 +75,16 @@ > */ > #define FF_CODEC_CAP_SETS_FRAME_PROPS (1 << 8) > > +#if CONFIG_OSSFUZZ > /** > * FFCodec.codec_tags termination value > */ > #define FF_CODEC_TAGS_END -1 > +#define FF_CODEC_TAGS(...) \ > +.codec_tags = (const uint32_t[]){ __VA_ARGS__, FF_CODEC_TAGS_END }, > +#else > +#define FF_CODEC_TAGS(...) > +#endif > > typedef struct FFCodecDefault { > const char *key; > @@ -196,10 +203,13 @@ typedef struct FFCodec { > */ > const struct AVCodecHWConfigInternal *const *hw_configs; > > +#if CONFIG_OSSFUZZ > /** > * List of supported codec_tags, terminated by FF_CODEC_TAGS_END. > + * Should be defined with the FF_CODEC_TAGS() macro. > */ > const uint32_t *codec_tags; > +#endif > } FFCodec; > > static av_always_inline const FFCodec *ffcodec(const AVCodec *codec) > diff --git a/libavcodec/hapdec.c b/libavcodec/hapdec.c > index 4a7ac15a8e..72f922bc5b 100644 > --- a/libavcodec/hapdec.c > +++ b/libavcodec/hapdec.c > @@ -486,12 +486,9 @@ const FFCodec ff_hap_decoder = { >AV_CODEC_CAP_DR1, > .caps_internal = FF_CODEC_CAP_INIT_THREADSAFE | >FF_CODEC_CAP_INIT_CLEANUP, > -.codec_tags = (const uint32_t []){ > -MKTAG('H','a','p','1'), > -MKTAG('H','a','p','5'), > -MKTAG('H','a','p','Y'), > -MKTAG('H','a','p','A'), > -MKTAG('H','a','p','M'), > -FF_CODEC_TAGS_END, > -}, > +FF_CODEC_TAGS(MKTAG('H','a','p','1'), > + MKTAG('H','a','p','5'), > + MKTAG('H','a','p','Y'), > + MKTAG('H','a','p','A'), > + MKTAG('H','a','p','M')) > }; > 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) > & 0x7FFF; > ctx->ch_layout.nb_channels = > (unsigned)bytestream2_get_le32(&gbc) % FF_SANE_NB_CHANNELS; > ctx->block_align= bytestream2_get_le32(&gbc) > & 0x7FFF; > +#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); > Will apply tomorrow unless there are objections. - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".