Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/3] h274: correctly infer missing comp model values
@ 2022-03-05 16:58 Niklas Haas
  2022-03-05 16:58 ` [FFmpeg-devel] [PATCH 2/3] h274: avoid copying AVFilmGrainH274Params into the stack frame Niklas Haas
  2022-03-05 16:58 ` [FFmpeg-devel] [PATCH 3/3] h274: support a few more pixfmts Niklas Haas
  0 siblings, 2 replies; 3+ messages in thread
From: Niklas Haas @ 2022-03-05 16:58 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Niklas Haas

From: Niklas Haas <git@haasn.dev>

The ITU-R H.274 spec mentions defined behavior for what to do when
num_comp_model_values is less than the amount expected by the fg model
algorithm.

The current behavior is basically undefined behavior in this case. In
addition to fixing this, also defer the index offset calculation very
slightly to better lead into the following commit.

Signed-off-by: Niklas Haas <git@haasn.dev>
---
 libavcodec/h274.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/h274.c b/libavcodec/h274.c
index a69f941142..170086543f 100644
--- a/libavcodec/h274.c
+++ b/libavcodec/h274.c
@@ -162,6 +162,7 @@ static av_always_inline void generate(int8_t *out, int out_stride,
 {
     const uint8_t shift = h274->log2_scale_factor + 6;
     const uint16_t avg = avg_8x8_c(in, in_stride);
+    const uint8_t num_values = h274->num_model_values[c];
     int16_t scale;
     uint8_t h, v;
     int8_t s = -1;
@@ -191,8 +192,8 @@ static av_always_inline void generate(int8_t *out, int out_stride,
         return;
     }
 
-    h = av_clip(h274->comp_model_value[c][s][1], 2, 14) - 2;
-    v = av_clip(h274->comp_model_value[c][s][2], 2, 14) - 2;
+    h = num_values > 1 ? av_clip(h274->comp_model_value[c][s][1], 2, 14) - 2 : 6;
+    v = num_values > 2 ? av_clip(h274->comp_model_value[c][s][2], 2, 14) - 2 : h;
     init_slice(database, h, v);
 
     scale = h274->comp_model_value[c][s][0];
-- 
2.35.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [FFmpeg-devel] [PATCH 2/3] h274: avoid copying AVFilmGrainH274Params into the stack frame
  2022-03-05 16:58 [FFmpeg-devel] [PATCH 1/3] h274: correctly infer missing comp model values Niklas Haas
@ 2022-03-05 16:58 ` Niklas Haas
  2022-03-05 16:58 ` [FFmpeg-devel] [PATCH 3/3] h274: support a few more pixfmts Niklas Haas
  1 sibling, 0 replies; 3+ messages in thread
From: Niklas Haas @ 2022-03-05 16:58 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Niklas Haas

From: Niklas Haas <git@haasn.dev>

There's very little reason to make a local copy of this entire ~10 kB
struct, only to precompute three minor arithmetic operations. Just move
the logic to the per-block function call instead.

Signed-off-by: Niklas Haas <git@haasn.dev>
---
 libavcodec/h274.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/libavcodec/h274.c b/libavcodec/h274.c
index 170086543f..265bd49ea1 100644
--- a/libavcodec/h274.c
+++ b/libavcodec/h274.c
@@ -192,14 +192,18 @@ static av_always_inline void generate(int8_t *out, int out_stride,
         return;
     }
 
-    h = num_values > 1 ? av_clip(h274->comp_model_value[c][s][1], 2, 14) - 2 : 6;
-    v = num_values > 2 ? av_clip(h274->comp_model_value[c][s][2], 2, 14) - 2 : h;
-    init_slice(database, h, v);
-
     scale = h274->comp_model_value[c][s][0];
     if (invert)
         scale = -scale;
+    if (c > 0)
+        scale >>= 1; // reduce intensity for chroma (as per SMPTE RDD 5-2006)
 
+    h = num_values > 1 ? h274->comp_model_value[c][s][1] : 8;
+    v = num_values > 2 ? h274->comp_model_value[c][s][2] : h;
+    h = av_clip(h << (c > 0 ? 1 : 0), 2, 14) - 2;
+    v = av_clip(v << (c > 0 ? 1 : 0), 2, 14) - 2;
+
+    init_slice(database, h, v);
     synth_grain_8x8_c(out, out_stride, scale, shift,
                       &database->db[h][v][y_offset][x_offset]);
 
@@ -219,9 +223,9 @@ int ff_h274_apply_film_grain(AVFrame *out_frame, const AVFrame *in_frame,
                              H274FilmGrainDatabase *database,
                              const AVFilmGrainParams *params)
 {
-    AVFilmGrainH274Params h274 = params->codec.h274;
+    const AVFilmGrainH274Params *h274 = &params->codec.h274;
     av_assert1(params->type == AV_FILM_GRAIN_PARAMS_H274);
-    if (h274.model_id != 0)
+    if (h274->model_id != 0)
         return AVERROR_PATCHWELCOME;
 
     av_assert1(out_frame->format == in_frame->format);
@@ -241,21 +245,12 @@ int ff_h274_apply_film_grain(AVFrame *out_frame, const AVFrame *in_frame,
         const uint8_t * const in = in_frame->data[c];
         const int in_stride = in_frame->linesize[c];
 
-        if (!h274.component_model_present[c]) {
+        if (!h274->component_model_present[c]) {
             av_image_copy_plane(out, out_stride, in, in_stride,
                                 width * sizeof(uint8_t), height);
             continue;
         }
 
-        if (c > 0) {
-            // Adaptation for 4:2:0 chroma subsampling
-            for (int i = 0; i < h274.num_intensity_intervals[c]; i++) {
-                h274.comp_model_value[c][i][0] >>= 1;
-                h274.comp_model_value[c][i][1] *= 2;
-                h274.comp_model_value[c][i][2] *= 2;
-            }
-        }
-
         // Film grain synthesis is done in 8x8 blocks, but the PRNG state is
         // only advanced in 16x16 blocks, so use a nested loop
         for (int y = 0; y < height; y += 16) {
@@ -271,7 +266,7 @@ int ff_h274_apply_film_grain(AVFrame *out_frame, const AVFrame *in_frame,
                     for (int xx = 0; xx < 16 && x+xx < width; xx += 8) {
                         generate(grain + (y+yy) * grain_stride + (x+xx), grain_stride,
                                  in + (y+yy) * in_stride + (x+xx), in_stride,
-                                 database, &h274, c, invert, (x+xx) > 0,
+                                 database, h274, c, invert, (x+xx) > 0,
                                  y_offset + yy, x_offset + xx);
                     }
                 }
-- 
2.35.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [FFmpeg-devel] [PATCH 3/3] h274: support a few more pixfmts
  2022-03-05 16:58 [FFmpeg-devel] [PATCH 1/3] h274: correctly infer missing comp model values Niklas Haas
  2022-03-05 16:58 ` [FFmpeg-devel] [PATCH 2/3] h274: avoid copying AVFilmGrainH274Params into the stack frame Niklas Haas
@ 2022-03-05 16:58 ` Niklas Haas
  1 sibling, 0 replies; 3+ messages in thread
From: Niklas Haas @ 2022-03-05 16:58 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Niklas Haas

From: Niklas Haas <git@haasn.dev>

Based on extrapolating the intent of the SMPTE RDD 5-2006 spec, which
only provides the parameter adaptation code for yuv420p. I've deduced
that the change to `scale` has nothing to do with the reduction in grain
frequency and merely reflects the fact that chroma planes have half the
effective value range in typical integer encodings.

As such, with this change can support e.g. 4:4:4 yuv420p as well, in
which case we simply don't double the chroma grain frequencies.

Still no support for 10-bit, but at least this is something.

Signed-off-by: Niklas Haas <git@haasn.dev>
---
 libavcodec/h274.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/libavcodec/h274.c b/libavcodec/h274.c
index 265bd49ea1..e800351a3e 100644
--- a/libavcodec/h274.c
+++ b/libavcodec/h274.c
@@ -157,7 +157,8 @@ static av_always_inline void generate(int8_t *out, int out_stride,
                                       const uint8_t *in, int in_stride,
                                       H274FilmGrainDatabase *database,
                                       const AVFilmGrainH274Params *h274,
-                                      int c, int invert, int deblock,
+                                      int c, uint8_t xs, uint8_t ys,
+                                      int invert, int deblock,
                                       int y_offset, int x_offset)
 {
     const uint8_t shift = h274->log2_scale_factor + 6;
@@ -200,8 +201,8 @@ static av_always_inline void generate(int8_t *out, int out_stride,
 
     h = num_values > 1 ? h274->comp_model_value[c][s][1] : 8;
     v = num_values > 2 ? h274->comp_model_value[c][s][2] : h;
-    h = av_clip(h << (c > 0 ? 1 : 0), 2, 14) - 2;
-    v = av_clip(v << (c > 0 ? 1 : 0), 2, 14) - 2;
+    h = av_clip(h << xs, 2, 14) - 2;
+    v = av_clip(v << ys, 2, 14) - 2;
 
     init_slice(database, h, v);
     synth_grain_8x8_c(out, out_stride, scale, shift,
@@ -224,19 +225,34 @@ int ff_h274_apply_film_grain(AVFrame *out_frame, const AVFrame *in_frame,
                              const AVFilmGrainParams *params)
 {
     const AVFilmGrainH274Params *h274 = &params->codec.h274;
+    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(out_frame->format);
     av_assert1(params->type == AV_FILM_GRAIN_PARAMS_H274);
     if (h274->model_id != 0)
         return AVERROR_PATCHWELCOME;
 
     av_assert1(out_frame->format == in_frame->format);
-    if (in_frame->format != AV_PIX_FMT_YUV420P)
+    switch (out_frame->format) {
+    case AV_PIX_FMT_YUV444P:
+    case AV_PIX_FMT_YUV440P:
+    case AV_PIX_FMT_YUV422P:
+    case AV_PIX_FMT_YUV420P:
+    case AV_PIX_FMT_YUV411P:
+    case AV_PIX_FMT_YUV410P:
+        // 8-bit YCbCr formats with varying subsampling
+        break;
+
+    default:
+        // TODO: support higher bit depth formats!
         return AVERROR_PATCHWELCOME;
+    }
 
     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];
-        const int width = c > 0 ? AV_CEIL_RSHIFT(out_frame->width, 1) : out_frame->width;
-        const int height = c > 0 ? AV_CEIL_RSHIFT(out_frame->height, 1) : out_frame->height;
+        const uint8_t xs = c > 0 ? desc->log2_chroma_w : 0;
+        const uint8_t ys = c > 0 ? desc->log2_chroma_h : 0;
+        const int width = AV_CEIL_RSHIFT(out_frame->width, xs);
+        const int height = AV_CEIL_RSHIFT(out_frame->height, ys);
 
         uint8_t * const out = out_frame->data[c];
         const int out_stride = out_frame->linesize[c];
@@ -266,7 +282,7 @@ int ff_h274_apply_film_grain(AVFrame *out_frame, const AVFrame *in_frame,
                     for (int xx = 0; xx < 16 && x+xx < width; xx += 8) {
                         generate(grain + (y+yy) * grain_stride + (x+xx), grain_stride,
                                  in + (y+yy) * in_stride + (x+xx), in_stride,
-                                 database, h274, c, invert, (x+xx) > 0,
+                                 database, h274, c, xs, ys, invert, (x+xx) > 0,
                                  y_offset + yy, x_offset + xx);
                     }
                 }
-- 
2.35.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-03-05 16:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-05 16:58 [FFmpeg-devel] [PATCH 1/3] h274: correctly infer missing comp model values Niklas Haas
2022-03-05 16:58 ` [FFmpeg-devel] [PATCH 2/3] h274: avoid copying AVFilmGrainH274Params into the stack frame Niklas Haas
2022-03-05 16:58 ` [FFmpeg-devel] [PATCH 3/3] h274: support a few more pixfmts Niklas Haas

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git