On 20/09/2024 09:42, Segall, Andrew wrote:


On 9/20/24, 12:15 AM, "ffmpeg-devel on behalf of Lynne via ffmpeg-devel" 
<ffmpeg-devel-boun...@ffmpeg.org <mailto:ffmpeg-devel-boun...@ffmpeg.org> on behalf of 
ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>> wrote:


CAUTION: This email originated from outside of the organization. Do not click 
links or open attachments unless you can confirm the sender and know the 
content is safe.






On 20/09/2024 02:44, Segall, Andrew via ffmpeg-devel wrote:
Details: Add support for the apply_grain_flag. This allows the film grain 
process to be enabled/disabled for different display properties.

On 9/8/24, 12:06 AM, "Andrew Segall" <aseg...@amazon.com <mailto:aseg...@amazon.com> 
<mailto:aseg...@amazon.com <mailto:aseg...@amazon.com>>> wrote:


Signed-off-by: Andrew Segall <aseg...@amazon.com <mailto:aseg...@amazon.com> 
<mailto:aseg...@amazon.com <mailto:aseg...@amazon.com>>>
---
libavcodec/aom_film_grain.c | 6 ++++--
libavcodec/hevc/hevcdec.c | 5 ++++-
libavfilter/vf_showinfo.c | 1 +
libavutil/film_grain_params.h | 5 +++++
4 files changed, 14 insertions(+), 3 deletions(-)


diff --git a/libavcodec/aom_film_grain.c b/libavcodec/aom_film_grain.c
index fdfa3dbf77..251a2793ac 100644
--- a/libavcodec/aom_film_grain.c
+++ b/libavcodec/aom_film_grain.c
@@ -149,8 +149,10 @@ int ff_aom_parse_film_grain_sets(AVFilmGrainAFGS1Params *s,
fgp = &s->sets[set_idx];
aom = &fgp->codec.aom;


- fgp->type = get_bits1(gb) ? AV_FILM_GRAIN_PARAMS_AV1 : 
AV_FILM_GRAIN_PARAMS_NONE;
- if (!fgp->type)
+ fgp->type = AV_FILM_GRAIN_PARAMS_AV1;
+
+ fgp->apply_grain = get_bits1(gb);
+ if ( !fgp->apply_grain)
{
payload_bits = get_bits_count(gb) - start_position;
if (payload_bits > payload_size * 8)
diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
index d915d74d22..8dd4fb10c5 100644
--- a/libavcodec/hevc/hevcdec.c
+++ b/libavcodec/hevc/hevcdec.c
@@ -3155,7 +3155,10 @@ static int hevc_frame_end(HEVCContext *s)
&s->h274db, fgp);
break;
case AV_FILM_GRAIN_PARAMS_AV1:
- ret = ff_aom_apply_film_grain(out->frame_grain, out->f, fgp);
+ if(fgp->apply_grain)
+ ret = ff_aom_apply_film_grain(out->frame_grain, out->f, fgp);
+ else
+ out->needs_fg = 0;
break;
}
av_assert1(ret >= 0);
diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
index f81df9d1bf..08e144ca46 100644
--- a/libavfilter/vf_showinfo.c
+++ b/libavfilter/vf_showinfo.c
@@ -455,6 +455,7 @@ static void 
dump_sei_film_grain_params_metadata(AVFilterContext *ctx, const AVFr
}


av_log(ctx, AV_LOG_INFO, "type %s; ", film_grain_type_names[fgp->type]);
+ av_log(ctx, AV_LOG_INFO, "apply_grain=%d; ", fgp->apply_grain);
av_log(ctx, AV_LOG_INFO, "seed=%"PRIu64"; ", fgp->seed);
av_log(ctx, AV_LOG_INFO, "width=%d; ", fgp->width);
av_log(ctx, AV_LOG_INFO, "height=%d; ", fgp->height);
diff --git a/libavutil/film_grain_params.h b/libavutil/film_grain_params.h
index ccacab88fe..f3275923e1 100644
--- a/libavutil/film_grain_params.h
+++ b/libavutil/film_grain_params.h
@@ -241,6 +241,11 @@ typedef struct AVFilmGrainParams {
*/
enum AVFilmGrainParamsType type;


+ /**
+ * Specifies if film grain parameters are enabled.
+ */
+ int apply_grain;
+
/**
* Seed to use for the synthesis process, if the codec allows for it.
*


This is an ABI break.

Furthermore, what exactly does this improve over simply stripping the side data?

Mainly addressing conformance issues.

Example is that we have two parameters in the AFGS1 message.  The first is 
associated with resolution A and apply_grain=1.  Second is associated with 
resolution B and apply_grain=0.

If the synthesis Is performed at resolution B, then the message says that we 
shouldn't perform film grain synthesis since apply_grain=0.  However, if we 
strip, the information is lost - and synthesis may use the parameters 
associated with resolution A instead.

Ah, I see, that makes sense.

You should put the new flag on the bottom, after bit_depth_chroma, to avoid breaking the ABI. You should also make sure that nothing gets broken. That includes adding apply = 1 in av_film_grain_params_create_side_data() to enable it by default, and making sure that all libavcodec users of the API are updated.

Also, better call it simply `apply`, its in a structure called AVFilmGrainParams after all.

Attachment: OpenPGP_0xA2FEA5F03F034464.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital 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".

Reply via email to