On 11/12/2020 4:28 PM, Lynne wrote:
Nov 12, 2020, 19:46 by jamr...@gmail.com:

On 11/12/2020 2:42 PM, Lynne wrote:

This patch introduces a new frame side data type AVFilmGrainParams for use
with video codecs which are able to use it.

It is generalized rather than being AV1 specific as AV2 is expected to carry
the same data, as well as the fact there already exist rarely-used 
specifications
for both H.264 and HEVC.

Patch attached.

 From 522e3a80f44129c98f0c564d44815dbe6a740568 Mon Sep 17 00:00:00 2001
From: Lynne <d...@lynne.ee>
Date: Thu, 12 Nov 2020 12:44:30 +0100
Subject: [PATCH v2 1/3] libavutil: introduce AVFilmGrainParams side data

This patch introduces a new frame side data type AVFilmGrainParams for use
with video codecs which are able to use it.

It is generalized rather than being AV1 specific as AV2 is expected to carry
the same data, as well as the fact there already exist rarely-used 
specifications
for both H.264 and HEVC.
---
  doc/APIchanges                |   4 +
  libavutil/Makefile            |   2 +
  libavutil/film_grain_params.c |  42 +++++++++
  libavutil/film_grain_params.h | 159 ++++++++++++++++++++++++++++++++++
  libavutil/frame.c             |   1 +
  libavutil/frame.h             |   6 ++
  libavutil/version.h           |   2 +-
  7 files changed, 215 insertions(+), 1 deletion(-)
  create mode 100644 libavutil/film_grain_params.c
  create mode 100644 libavutil/film_grain_params.h

diff --git a/doc/APIchanges b/doc/APIchanges
index b70c78a483..41248724d9 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,10 @@ libavutil:     2017-10-21
  API changes, most recent first:
  +2020-xx-xx - xxxxxxxxxx - lavu 56.61.100 - film_grain_params.h
+  Adds a new API for extracting codec film grain parameters as side data.
+  Adds a new AVFrameSideDataType entry AV_FRAME_DATA_FILM_GRAIN_PARAMS for it.
+
  2020-xx-xx - xxxxxxxxxx - lavf 58.64.100 - avformat.h
  Add AVSTREAM_EVENT_FLAG_NEW_PACKETS.
  diff --git a/libavutil/Makefile b/libavutil/Makefile
index 9b08372eb2..27bafe9e12 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -84,6 +84,7 @@ HEADERS = adler32.h                                           
          \
  xtea.h                                                        \
  tea.h                                                         \
  tx.h                                                          \
+          film_grain_params.h                                           \
  HEADERS-$(CONFIG_LZO)                   += lzo.h
  @@ -170,6 +171,7 @@ OBJS = adler32.o                                          
              \
  tx_double.o                                                      \
  tx_int32.o                                                       \
  video_enc_params.o                                               \
+       film_grain_params.o                                              \
  OBJS-$(CONFIG_CUDA)                     += hwcontext_cuda.o
diff --git a/libavutil/film_grain_params.c b/libavutil/film_grain_params.c
new file mode 100644
index 0000000000..930d23c7fe
--- /dev/null
+++ b/libavutil/film_grain_params.c
@@ -0,0 +1,42 @@
+/**
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "film_grain_params.h"
+
+AVFilmGrainParams *av_film_grain_params_alloc(size_t *size)
+{
+    AVFilmGrainParams *params = av_mallocz(sizeof(AVFilmGrainParams));
+
+    if (size)
+        *size = sizeof(*params);
+
+    return params;
+}
+
+AVFilmGrainParams *av_film_grain_params_create_side_data(AVFrame *frame)
+{
+    AVFrameSideData *side_data = av_frame_new_side_data(frame,
+                                                        
AV_FRAME_DATA_FILM_GRAIN_PARAMS,
+                                                        
sizeof(AVFilmGrainParams));
+    if (!side_data)
+        return NULL;
+
+    memset(side_data->data, 0, sizeof(AVFilmGrainParams));
+
+    return (AVFilmGrainParams *)side_data->data;
+}
diff --git a/libavutil/film_grain_params.h b/libavutil/film_grain_params.h
new file mode 100644
index 0000000000..ba20fe7fa6
--- /dev/null
+++ b/libavutil/film_grain_params.h
@@ -0,0 +1,159 @@
+/*
+ * Copyright (c) 2016 Neil Birkbeck <neil.birkb...@gmail.com>


Copy paste mistake?


Yup.



+typedef struct AVFilmGrainAOMParams {


If you believe AV2 will have a compatible implementation, then this name is 
fine.


I don't think they're going to change it at all. They haven't even proven all 
this
system is capable of in order to replace it with something better, so they might
just do some minor tweaks. After all, its already somewhat ossified thanks to
hardware.



+    int8_t ar_coeffs_uv[2][25 + 3 /* padding for alignment purposes */];

Shouldn't the compiler take care of alignment?


This simplified dav1d's SIMD, hence why its there, so I decided to leave it 
as-is.
The comment above says so too, but this small comment draws too much attention,
so I removed it.

I don't think that's something we should consider in our public API. We should prioritize small portable structs, and not trying to optimize fields for one potential use in one potential target out of many.

This applies to every other case where you gave SIMD as reason.




+
+    /**
+     * Specifies the range of the auto-regressive coefficients. Values of 6,
+     * 7, 8 and so on represent a range of [-2, 2), [-1, 1), [-0.5, 0.5) and
+     * so on. For AV1 must be between 6 and 9.
+     */
+    uint64_t ar_coeff_shift;


Why uint64_t? Sounds like it may be excessive for this.


Same as above, simplifies SIMD. Directly used as a psrad memory arg.



+
+    /**
+     * Signals the down shift applied to the generated gaussian numbers during
+     * synthesis.
+     */
+    int grain_scale_shift;
+
+    /**
+     * Specifies the luma/chroma multipliers for the index to the component
+     * scaling function.
+     */
+    int uv_mult[2];
+    int uv_mult_luma[2];


int8_t, like ar_coeffs_y. Or could AV2 increase the range considerably?


Same as above, SIMD.



+
+    /**
+     * Offset used for component scaling function. For AV1 its a 9-bit value
+     * with a range [-256, 255] */
+    int uv_offset[2];


int16_t.


Could be changed, but there's a single movd in dav1d's SSSE3 SIMD.
Not sure if it worth it.



+
+    /**
+     * Signals whether to overlap film grain blocks.
+     */
+    int overlap_flag;
+} AVFilmGrainAOMParams;
+
+typedef struct AVFilmGrainParams {
+    /**
+     * Specifies the codec for which this structure is valid.
+     */
+    enum AVFilmGrainParamsType type;
+
+    /**
+     * Seed to use for the synthesis process, if the codec allows for it.
+     */
+    uint32_t seed;


Maybe uint64_t, just in case. This is meant to apply to all implementations.


Done.

Both changes applied locally.
_______________________________________________
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".


_______________________________________________
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