[FFmpeg-devel] [PATCH 2/2] avcodec/h264dec: Check early whether film grain is supported, fix race
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Sun Sep 17 03:15:26 EEST 2023
Applying film grain happens after ff_thread_finish_setup(),
so the parameters synced in ff_h264_update_thread_context()
must not be modified. But this is exactly what happens if
applying film grain fails. (The likely result is that in
case of frame threading an uninitialized frame is output.)
Failure of applying film grain can have several reasons:
The first is that it is just not supported (we only support
a specific model_id and only YUV420p). The second is that
no film grain side data is added to the AVFrame. This seems
possible even with legal input, namely if the second field
overrides (i.e. present equal to zero) an earlier film grain SEI.
Given that it is actually very easy to know in advance whether
ff_h274_apply_film_grain() supports a given set of parameters,
one can check for this before ff_thread_finish_setup()
and avoid allocating an unused buffer lateron.
The second failure condition has been addressed by keeping
track of the film grain state in a more granular fashion,
namely by distinguishing the cases of an allocated film
grain frame buffer and the existence of the side data.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
---
Honestly, I have no sample to test this (contrary to the preceding
HEVC patch). The only thing I could test is that it does not
break samples without film grain.
Of course, testing this on the sample that led to
762e18da3fe64dbe7d3091fddf99aeee164017cc is appropriate.
libavcodec/h2645_sei.c | 5 ++++-
libavcodec/h2645_sei.h | 2 +-
libavcodec/h264_picture.c | 23 +++++++++--------------
libavcodec/h264_slice.c | 28 ++++++++++++++++++++++++----
libavcodec/h264dec.c | 9 ++++++---
libavcodec/h264dec.h | 11 ++++++++++-
libavcodec/hevcdec.c | 2 +-
7 files changed, 55 insertions(+), 25 deletions(-)
diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
index cb6be0594b..3074c1eb73 100644
--- a/libavcodec/h2645_sei.c
+++ b/libavcodec/h2645_sei.c
@@ -512,7 +512,7 @@ int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei,
enum AVCodecID codec_id,
AVCodecContext *avctx, const H2645VUI *vui,
unsigned bit_depth_luma, unsigned bit_depth_chroma,
- int seed)
+ int seed, int *added_film_grain_sei)
{
H2645SEIFramePacking *fp = &sei->frame_packing;
@@ -686,6 +686,9 @@ int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei,
else
fgc->present = fgc->persistence_flag;
+ if (added_film_grain_sei)
+ *added_film_grain_sei = 1;
+
if (avctx)
avctx->properties |= FF_CODEC_PROPERTY_FILM_GRAIN;
}
diff --git a/libavcodec/h2645_sei.h b/libavcodec/h2645_sei.h
index 0ebf48011a..409b2028f8 100644
--- a/libavcodec/h2645_sei.h
+++ b/libavcodec/h2645_sei.h
@@ -163,6 +163,6 @@ int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei,
enum AVCodecID codec_id,
AVCodecContext *avctx, const H2645VUI *vui,
unsigned bit_depth_luma, unsigned bit_depth_chroma,
- int seed);
+ int seed, int *added_film_grain_sei);
#endif /* AVCODEC_H2645_SEI_H */
diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c
index 31b5e231c2..b27bcdf3d8 100644
--- a/libavcodec/h264_picture.c
+++ b/libavcodec/h264_picture.c
@@ -89,7 +89,7 @@ static void h264_copy_picture_params(H264Picture *dst, const H264Picture *src)
dst->mb_width = src->mb_width;
dst->mb_height = src->mb_height;
dst->mb_stride = src->mb_stride;
- dst->needs_fg = src->needs_fg;
+ dst->fg_status = src->fg_status;
}
int ff_h264_ref_picture(H264Context *h, H264Picture *dst, H264Picture *src)
@@ -105,7 +105,7 @@ int ff_h264_ref_picture(H264Context *h, H264Picture *dst, H264Picture *src)
if (ret < 0)
goto fail;
- if (src->needs_fg) {
+ if (src->fg_status != NO_FILM_GRAIN) {
ret = av_frame_ref(dst->f_grain, src->f_grain);
if (ret < 0)
goto fail;
@@ -165,7 +165,7 @@ int ff_h264_replace_picture(H264Context *h, H264Picture *dst, const H264Picture
if (ret < 0)
goto fail;
- if (src->needs_fg) {
+ if (src->fg_status != NO_FILM_GRAIN) {
ff_thread_release_buffer(h->avctx, dst->f_grain);
ret = av_frame_ref(dst->f_grain, src->f_grain);
if (ret < 0)
@@ -248,19 +248,14 @@ int ff_h264_field_end(H264Context *h, H264SliceContext *sl, int in_setup)
if (err < 0)
av_log(avctx, AV_LOG_ERROR,
"hardware accelerator failed to decode picture\n");
- } else if (!in_setup && cur->needs_fg && (!FIELD_PICTURE(h) || !h->first_field)) {
+ } else if (!in_setup && cur->fg_status == FILM_GRAIN_APPLICABLE &&
+ (!FIELD_PICTURE(h) || !h->first_field)) {
AVFrameSideData *sd = av_frame_get_side_data(cur->f, AV_FRAME_DATA_FILM_GRAIN_PARAMS);
+ av_unused int ret;
- err = AVERROR_INVALIDDATA;
- if (sd) // a decoding error may have happened before the side data could be allocated
- err = ff_h274_apply_film_grain(cur->f_grain, cur->f, &h->h274db,
- (AVFilmGrainParams *) sd->data);
- if (err < 0) {
- av_log(h->avctx, AV_LOG_WARNING, "Failed synthesizing film "
- "grain, ignoring: %s\n", av_err2str(err));
- cur->needs_fg = 0;
- err = 0;
- }
+ ret = ff_h274_apply_film_grain(cur->f_grain, cur->f, &h->h274db,
+ (AVFilmGrainParams *) sd->data);
+ av_assert1(ret >= 0);
}
if (!in_setup && !h->droppable)
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 5657327f0c..b450561b49 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -196,7 +196,7 @@ static int alloc_picture(H264Context *h, H264Picture *pic)
if (ret < 0)
goto fail;
- if (pic->needs_fg) {
+ if (pic->fg_status != NO_FILM_GRAIN) {
pic->f_grain->format = pic->f->format;
pic->f_grain->width = pic->f->width;
pic->f_grain->height = pic->f->height;
@@ -431,6 +431,8 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
h->workaround_bugs = h1->workaround_bugs;
h->droppable = h1->droppable;
+ h->film_grain_warning_shown = h1->film_grain_warning_shown;
+
// extradata/NAL handling
h->is_avc = h1->is_avc;
h->nal_length_size = h1->nal_length_size;
@@ -543,8 +545,18 @@ FF_ENABLE_DEPRECATION_WARNINGS
pic->f->crop_top = h->crop_top;
pic->f->crop_bottom = h->crop_bottom;
- pic->needs_fg = h->sei.common.film_grain_characteristics.present && !h->avctx->hwaccel &&
- !(h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN);
+ pic->fg_status = h->sei.common.film_grain_characteristics.present &&
+ !h->avctx->hwaccel &&
+ !(h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN) ?
+ FILM_GRAIN_INTENDED : NO_FILM_GRAIN;
+
+ if (pic->fg_status != NO_FILM_GRAIN &&
+ !ff_h274_film_grain_params_supported(h->sei.common.film_grain_characteristics.model_id,
+ pic->f->format)) {
+ av_log_once(h->avctx, AV_LOG_WARNING, AV_LOG_DEBUG, &h->film_grain_warning_shown,
+ "Unsupported film grain parameters. Ignoring film grain.\n");
+ pic->fg_status = NO_FILM_GRAIN;
+ }
if ((ret = alloc_picture(h, pic)) < 0)
return ret;
@@ -1191,6 +1203,7 @@ static int h264_export_frame_props(H264Context *h)
H264Picture *cur = h->cur_pic_ptr;
AVFrame *out = cur->f;
int interlaced_frame = 0, top_field_first = 0;
+ int added_film_grain_sei = 0;
int ret;
out->flags &= ~AV_FRAME_FLAG_INTERLACED;
@@ -1274,10 +1287,17 @@ static int h264_export_frame_props(H264Context *h)
ret = ff_h2645_sei_to_frame(out, &h->sei.common, AV_CODEC_ID_H264, h->avctx,
&sps->vui, sps->bit_depth_luma, sps->bit_depth_chroma,
- cur->poc + (unsigned)(h->poc_offset << 5));
+ cur->poc + (unsigned)(h->poc_offset << 5),
+ &added_film_grain_sei);
if (ret < 0)
return ret;
+ // If the first field's film grain side data is later overridden
+ // in the second field, we may end up with f_grain allocated without
+ // frame side data (so no film grain can be applied).
+ if (cur->fg_status != NO_FILM_GRAIN && added_film_grain_sei)
+ cur->fg_status = FILM_GRAIN_APPLICABLE;
+
if (h->sei.picture_timing.timecode_cnt > 0) {
uint32_t *tc_sd;
char tcbuf[AV_TIMECODE_STR_SIZE];
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 796f80be8d..b4e0bf8507 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -870,12 +870,15 @@ static int output_frame(H264Context *h, AVFrame *dst, H264Picture *srcp)
{
int ret;
- ret = av_frame_ref(dst, srcp->needs_fg ? srcp->f_grain : srcp->f);
+ ret = av_frame_ref(dst, srcp->fg_status == FILM_GRAIN_APPLICABLE ? srcp->f_grain : srcp->f);
if (ret < 0)
return ret;
- if (srcp->needs_fg && (ret = av_frame_copy_props(dst, srcp->f)) < 0)
- return ret;
+ if (srcp->fg_status == FILM_GRAIN_APPLICABLE) {
+ ret = av_frame_copy_props(dst, srcp->f);
+ if (ret < 0)
+ return ret;
+ }
if (srcp->decode_error_flags) {
atomic_int *decode_error = (atomic_int*)srcp->decode_error_flags->data;
diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
index 322c06a19c..58b6996a04 100644
--- a/libavcodec/h264dec.h
+++ b/libavcodec/h264dec.h
@@ -145,7 +145,14 @@ typedef struct H264Picture {
int recovered; ///< picture at IDR or recovery point + recovery count
int invalid_gap;
int sei_recovery_frame_cnt;
- int needs_fg; ///< whether picture needs film grain synthesis (see `f_grain`)
+ enum {
+ /**
+ * Either film grain not present or applying not intended or impossible
+ */
+ NO_FILM_GRAIN = 0,
+ FILM_GRAIN_INTENDED, ///< film grain found and buffer has been allocated
+ FILM_GRAIN_APPLICABLE, ///< film grain buffer and side data allocated
+ } fg_status;
AVBufferRef *pps_buf;
const PPS *pps;
@@ -537,6 +544,8 @@ typedef struct H264Context {
int cur_bit_depth_luma;
int16_t slice_row[MAX_SLICES]; ///< to detect when MAX_SLICES is too low
+ int film_grain_warning_shown;
+
/* original AVCodecContext dimensions, used to handle container
* cropping */
int width_from_caller;
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 2be62ddfb2..65581c956c 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -2778,7 +2778,7 @@ static int set_side_data(HEVCContext *s)
ret = ff_h2645_sei_to_frame(out, &s->sei.common, AV_CODEC_ID_HEVC, NULL,
&s->ps.sps->vui.common,
s->ps.sps->bit_depth, s->ps.sps->bit_depth_chroma,
- s->ref->poc /* no poc_offset in HEVC */);
+ s->ref->poc /* no poc_offset in HEVC */, NULL);
if (ret < 0)
return ret;
--
2.34.1
More information about the ffmpeg-devel
mailing list