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