PR #24926 opened by dalecurtis URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24926 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24926.patch
In Chrome-branded builds, libavcodec/h274.c places 700,440 bytes of film grain buffers in chrome.dll's .bss (`film_grain_db` at 692,248 B and `init_slice_c::tmp` at 8,192 B), even though these buffers are only touched when software H.264 decoding encounters an H.274 Film Grain Characteristics SEI. On Windows, the PE loader pre-commits chrome.dll's entire .bss in every process. Move `init_slice_c::tmp` into `H274FilmGrainDatabase` (700,440 B total) and allocate `film_grain_db` on the heap via `ff_thread_once()` on the first call to `ff_h274_apply_film_grain()`. This is safe because: - `ff_h274_apply_film_grain()` is the sole entry point that accesses `film_grain_db`. Its callers in `h264_picture.c` and `vvc/dec.c` already handle negative error returns (`AVERROR(ENOMEM)`), and this CL updates `hevc_frame_end()` in `hevc/hevcdec.c` to propagate negative returns instead of asserting `ret >= 0`. - `init_slice_c()` is only called while holding `init_slice()`'s static mutex, so storing `tmp` in `H274FilmGrainDatabase` preserves the same thread safety without a large stack allocation. Impact (Windows x64 official Chrome-branded chrome.dll): - .data VirtualSize: 3,758,744 B -> 3,058,328 B (-684 KiB / ~-9.1 MiB at p50 process count). - Binary file size (.text/.data RawSize): +0 B. >From b3b69b9f0e0d94a73c81a1c1a5793fafad4693e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Doray?= <[email protected]> Date: Wed, 7 Oct 2026 14:50:06 +0000 Subject: [PATCH] Heap-allocate H.274 film grain tables in h274.c on first use In Chrome-branded builds, libavcodec/h274.c places 700,440 bytes of film grain buffers in chrome.dll's .bss (`film_grain_db` at 692,248 B and `init_slice_c::tmp` at 8,192 B), even though these buffers are only touched when software H.264 decoding encounters an H.274 Film Grain Characteristics SEI. On Windows, the PE loader pre-commits chrome.dll's entire .bss in every process. Move `init_slice_c::tmp` into `H274FilmGrainDatabase` (700,440 B total) and allocate `film_grain_db` on the heap via `ff_thread_once()` on the first call to `ff_h274_apply_film_grain()`. This is safe because: - `ff_h274_apply_film_grain()` is the sole entry point that accesses `film_grain_db`. Its callers in `h264_picture.c` and `vvc/dec.c` already handle negative error returns (`AVERROR(ENOMEM)`), and this CL updates `hevc_frame_end()` in `hevc/hevcdec.c` to propagate negative returns instead of asserting `ret >= 0`. - `init_slice_c()` is only called while holding `init_slice()`'s static mutex, so storing `tmp` in `H274FilmGrainDatabase` preserves the same thread safety without a large stack allocation. Impact (Windows x64 official Chrome-branded chrome.dll): - .data VirtualSize: 3,758,744 B -> 3,058,328 B (-684 KiB / ~-9.1 MiB at p50 process count). - Binary file size (.text/.data RawSize): +0 B. --- libavcodec/h274.c | 44 +++++++++++++++++++++++++++++++-------- libavcodec/h274.h | 10 ++++----- libavcodec/hevc/hevcdec.c | 10 +++++++-- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/libavcodec/h274.c b/libavcodec/h274.c index e82e31489e..5aaf7e861c 100644 --- a/libavcodec/h274.c +++ b/libavcodec/h274.c @@ -42,9 +42,28 @@ typedef struct H274FilmGrainDatabase { // Database of film grain patterns, lazily computed as-needed int8_t db[13 /* h */][13 /* v */][64][64]; atomic_uint residency[6]; + // Chromium: Move init_slice_c's temporary buffer (8,192 bytes) here and + // heap-allocate H274FilmGrainDatabase (700,440 bytes total on all + // architectures with 32-bit atomic_uint) on first use instead of keeping it + // in .bss. + int16_t tmp[64][64]; } H274FilmGrainDatabase; -static H274FilmGrainDatabase film_grain_db; +static H274FilmGrainDatabase *film_grain_db; +static AVOnce film_grain_db_once = AV_ONCE_INIT; + +static av_cold void init_film_grain_db(void) +{ + // Only `residency` (24 bytes with 32-bit atomic_uint) needs + // zero-initialization (`init_slice_c` fully writes each `db[h][v]` slice + // and `tmp` before reading them), so use `av_malloc` instead of + // `av_mallocz` to avoid faulting in and dirtying all 700,440 bytes upfront. + film_grain_db = av_malloc(sizeof(*film_grain_db)); + if (film_grain_db) { + for (size_t i = 0; i < FF_ARRAY_ELEMS(film_grain_db->residency); i++) + atomic_init(&film_grain_db->residency[i], 0); + } +} static const int8_t Gaussian_LUT[2048+4]; static const uint32_t Seed_LUT[256]; @@ -67,9 +86,9 @@ static void init_slice_c(int8_t out[64][64], uint8_t h, uint8_t v) const uint8_t deblock_coeff = deblock_factors[v]; const uint8_t freq_h = ((h + 3) << 2) - 1; const uint8_t freq_v = ((v + 3) << 2) - 1; - // Temporary buffer for slice generation - // FIXME: Static or not? - static int16_t tmp[64][64]; + // Temporary buffer for slice generation (protected by init_slice's mutex). + // Chromium: Stored in heap-allocated film_grain_db instead of static .bss. + int16_t (*tmp)[64] = film_grain_db->tmp; uint32_t seed = Seed_LUT[h + v * 13]; // Initialize with random gaussian values, using the output array as a @@ -123,17 +142,17 @@ static void init_slice(uint8_t h, uint8_t v) { static AVMutex mutex = AV_MUTEX_INITIALIZER; unsigned bitpos = h * 13 + v; - unsigned res = atomic_load_explicit(&film_grain_db.residency[bitpos / 32], + unsigned res = atomic_load_explicit(&film_grain_db->residency[bitpos / 32], memory_order_acquire); if (res & (1U << (bitpos & 31))) return; ff_mutex_lock(&mutex); - res = atomic_load_explicit(&film_grain_db.residency[bitpos / 32], memory_order_relaxed); + res = atomic_load_explicit(&film_grain_db->residency[bitpos / 32], memory_order_relaxed); if (!(res & (1U << (bitpos & 31)))) { - init_slice_c(film_grain_db.db[h][v], h, v); - atomic_store_explicit(&film_grain_db.residency[bitpos / 32], + init_slice_c(film_grain_db->db[h][v], h, v); + atomic_store_explicit(&film_grain_db->residency[bitpos / 32], res | (1U << (bitpos & 31)), memory_order_release); } ff_mutex_unlock(&mutex); @@ -228,7 +247,7 @@ static av_always_inline void generate(int8_t *out, int out_stride, scale = -scale; synth_grain_8x8_c(out, out_stride, scale, shift, - &film_grain_db.db[h][v][y_offset][x_offset]); + &film_grain_db->db[h][v][y_offset][x_offset]); if (deblock) deblock_8x8_c(out, out_stride); @@ -254,6 +273,13 @@ int ff_h274_apply_film_grain(AVFrame *out_frame, const AVFrame *in_frame, if (in_frame->format != AV_PIX_FMT_YUV420P) return AVERROR_PATCHWELCOME; + // Chromium: Allocate film_grain_db on first use. On OOM, film_grain_db + // stays NULL for the lifetime of the process and AVERROR(ENOMEM) is + // returned. + ff_thread_once(&film_grain_db_once, init_film_grain_db); + if (!film_grain_db) + return AVERROR(ENOMEM); + for (int c = 0; c < 3; c++) { static const uint8_t color_offset[3] = { 0, 85, 170 }; uint32_t seed = Seed_LUT[(params->seed + color_offset[c]) % 256]; diff --git a/libavcodec/h274.h b/libavcodec/h274.h index 7bbc3e8aa7..e50a39c701 100644 --- a/libavcodec/h274.h +++ b/libavcodec/h274.h @@ -45,11 +45,11 @@ static inline int ff_h274_film_grain_params_supported(int model_id, enum AVPixel // must already have been allocated and set to the same size and format as // `in`. // -// Returns a negative error code on error, such as invalid params. -// If ff_h274_film_grain_params_supported() indicated that the parameters -// are supported, no error will be returned if the arguments given to -// ff_h274_film_grain_params_supported() coincide with actual values -// from the frames and params. +// Returns a negative error code on error, such as invalid params or +// AVERROR(ENOMEM). If ff_h274_film_grain_params_supported() indicated that the +// parameters are supported and the arguments given to +// ff_h274_film_grain_params_supported() coincide with actual values from the +// frames and params, only AVERROR(ENOMEM) can be returned. int ff_h274_apply_film_grain(AVFrame *out, const AVFrame *in, const AVFilmGrainParams *params); diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c index b6234a6c34..39009488ed 100644 --- a/libavcodec/hevc/hevcdec.c +++ b/libavcodec/hevc/hevcdec.c @@ -3525,7 +3525,7 @@ static int hevc_frame_end(HEVCContext *s, HEVCLayerContext *l) { HEVCFrame *out = l->cur_frame; const AVFilmGrainParams *fgp; - av_unused int ret; + int ret; if (out->needs_fg) { av_assert0(out->frame_grain->buf[0]); @@ -3541,7 +3541,13 @@ static int hevc_frame_end(HEVCContext *s, HEVCLayerContext *l) ret = ff_aom_apply_film_grain(out->frame_grain, out->f, fgp); break; } - av_assert1(ret >= 0); + // Chromium: Handle negative error returns (e.g. AVERROR(ENOMEM) from + // lazy film_grain_db allocation in ff_h274_apply_film_grain). + if (ret < 0) { + // Fall back to outputting `out->f` instead of `out->frame_grain`. + out->needs_fg = 0; + return ret; + } } if (s->avctx->hwaccel) { -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
