* [FFmpeg-devel] [PATCH 2/6] avutil/dovi_meta: add dolby vision extension blocks
2024-03-23 17:37 [FFmpeg-devel] [PATCH 1/6] avutil/dovi_meta: add AVDOVIDataMapping.nlq_pivots Niklas Haas
@ 2024-03-23 17:37 ` Niklas Haas
2024-03-23 17:58 ` Andreas Rheinhardt
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 3/6] avcodec/dovi_rpu: strip container in separate step Niklas Haas
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Niklas Haas @ 2024-03-23 17:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
As well as accessors plus a function for allocating this struct with
extension blocks,
Definitions generously taken from quietvoid/dovi_tool, which is
assembled as a collection of various patent fragments, as well as output
by the official Dolby Vision bitstream verifier tool.
---
doc/APIchanges | 5 ++
libavutil/dovi_meta.c | 36 ++++++++--
libavutil/dovi_meta.h | 154 ++++++++++++++++++++++++++++++++++++++++++
libavutil/version.h | 2 +-
4 files changed, 190 insertions(+), 7 deletions(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 739f33501e9..8b59150dc1f 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,11 @@ The last version increases of all libraries were on 2024-03-07
API changes, most recent first:
+2024-03-xx - xxxxxxxxxx - lavu 59.6.100 - dovi_meta.h
+ Add AVDOVIMetadata.ext_block_{offset,size}, AVDOVIMetadata.num_ext_blocks,
+ AVDOVIDmData and AVDOVIDmLevel{1..6,8..11,254..255}, av_dovi_get_ext(),
+ av_dovi_find_level() and av_dovi_metadata_alloc_ext(),
+
2024-03-xx - xxxxxxxxxx - lavu 59.5.100 - dovi_meta.h
Add AVDOVIDataMapping.nlq_pivots.
diff --git a/libavutil/dovi_meta.c b/libavutil/dovi_meta.c
index 9c50da561ed..17e18bf95f9 100644
--- a/libavutil/dovi_meta.c
+++ b/libavutil/dovi_meta.c
@@ -18,6 +18,8 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include <string.h>
+
#include "dovi_meta.h"
#include "mem.h"
@@ -39,22 +41,44 @@ typedef struct AVDOVIMetadataInternal {
AVDOVIRpuDataHeader header;
AVDOVIDataMapping mapping;
AVDOVIColorMetadata color;
+ AVDOVIDmData ext_blocks[];
} AVDOVIMetadataInternal;
-AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size)
+AVDOVIMetadata *av_dovi_metadata_alloc_ext(int num_ext_blocks, size_t *size)
{
- AVDOVIMetadataInternal *dovi = av_mallocz(sizeof(AVDOVIMetadataInternal));
+ const size_t alloc_size = sizeof(AVDOVIMetadataInternal) +
+ num_ext_blocks * sizeof(AVDOVIDmData);
+
+ AVDOVIMetadataInternal *dovi = av_mallocz(alloc_size);
if (!dovi)
return NULL;
if (size)
- *size = sizeof(*dovi);
+ *size = alloc_size;
dovi->metadata = (struct AVDOVIMetadata) {
- .header_offset = offsetof(AVDOVIMetadataInternal, header),
- .mapping_offset = offsetof(AVDOVIMetadataInternal, mapping),
- .color_offset = offsetof(AVDOVIMetadataInternal, color),
+ .header_offset = offsetof(AVDOVIMetadataInternal, header),
+ .mapping_offset = offsetof(AVDOVIMetadataInternal, mapping),
+ .color_offset = offsetof(AVDOVIMetadataInternal, color),
+ .ext_block_offset = offsetof(AVDOVIMetadataInternal, ext_blocks),
+ .ext_block_size = sizeof(AVDOVIDmData),
};
return &dovi->metadata;
}
+
+AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size)
+{
+ return av_dovi_metadata_alloc_ext(0, size);
+}
+
+AVDOVIDmData *av_dovi_find_level(const AVDOVIMetadata *data, uint8_t level)
+{
+ for (int i = 0; i < data->num_ext_blocks; i++) {
+ AVDOVIDmData *ext = av_dovi_get_ext(data, i);
+ if (ext->level == level)
+ return ext;
+ }
+
+ return NULL;
+}
diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
index 6afc7b055a7..e77d9853f67 100644
--- a/libavutil/dovi_meta.h
+++ b/libavutil/dovi_meta.h
@@ -29,7 +29,9 @@
#include <stdint.h>
#include <stddef.h>
+
#include "rational.h"
+#include "csp.h"
/*
* DOVI configuration
@@ -187,6 +189,130 @@ typedef struct AVDOVIColorMetadata {
uint16_t source_diagonal;
} AVDOVIColorMetadata;
+typedef struct AVDOVIDmLevel1 {
+ /* Per-frame brightness metadata */
+ uint16_t min_pq;
+ uint16_t max_pq;
+ uint16_t avg_pq;
+} AVDOVIDmLevel1;
+
+typedef struct AVDOVIDmLevel2 {
+ /* Usually derived from level 8 (at different levels) */
+ uint16_t target_max_pq;
+ uint16_t trim_slope;
+ uint16_t trim_offset;
+ uint16_t trim_power;
+ uint16_t trim_chroma_weight;
+ uint16_t trim_saturation_gain;
+ int16_t ms_weight;
+} AVDOVIDmLevel2;
+
+typedef struct AVDOVIDmLevel3 {
+ uint16_t min_pq_offset;
+ uint16_t max_pq_offset;
+ uint16_t avg_pq_offset;
+} AVDOVIDmLevel3;
+
+typedef struct AVDOVIDmLevel4 {
+ uint16_t anchor_pq;
+ uint16_t anchor_power;
+} AVDOVIDmLevel4;
+
+typedef struct AVDOVIDmLevel5 {
+ /* Active area definition */
+ uint16_t left_offset;
+ uint16_t right_offset;
+ uint16_t top_offset;
+ uint16_t bottom_offset;
+} AVDOVIDmLevel5;
+
+typedef struct AVDOVIDmLevel6 {
+ /* Static HDR10 metadata */
+ uint16_t max_luminance;
+ uint16_t min_luminance;
+ uint16_t max_cll;
+ uint16_t max_fall;
+} AVDOVIDmLevel6;
+
+typedef struct AVDOVIDmLevel8 {
+ /* Extended version of level 2 */
+ uint8_t target_display_index;
+ uint16_t trim_slope;
+ uint16_t trim_offset;
+ uint16_t trim_power;
+ uint16_t trim_chroma_weight;
+ uint16_t trim_saturation_gain;
+ uint16_t ms_weight;
+ uint16_t target_mid_contrast;
+ uint16_t clip_trim;
+ uint8_t saturation_vector_field[6];
+ uint8_t hue_vector_field[6];
+} AVDOVIDmLevel8;
+
+typedef struct AVDOVIDmLevel9 {
+ /* Source display characteristics */
+ uint8_t source_primary_index;
+ AVColorPrimariesDesc source_display_primaries;
+} AVDOVIDmLevel9;
+
+typedef struct AVDOVIDmLevel10 {
+ /* Target display characteristics */
+ uint8_t target_display_index;
+ uint16_t target_max_pq;
+ uint16_t target_min_pq;
+ uint8_t target_primary_index;
+ AVColorPrimariesDesc target_display_primaries;
+} AVDOVIDmLevel10;
+
+typedef struct AVDOVIDmLevel11 {
+ uint8_t content_type;
+ uint8_t whitepoint;
+ uint8_t reference_mode_flag;
+ uint8_t sharpness;
+ uint8_t noise_reduction;
+ uint8_t mpeg_noise_reduction;
+ uint8_t frame_rate_conversion;
+ uint8_t brightness;
+ uint8_t color;
+} AVDOVIDmLevel11;
+
+typedef struct AVDOVIDmLevel254 {
+ /* DMv2 info block, always present in samples with DMv2 metadata */
+ uint8_t dm_mode;
+ uint8_t dm_version_index;
+} AVDOVIDmLevel254;
+
+typedef struct AVDOVIDmLevel255 {
+ /* Debug block, not really used in samples */
+ uint8_t dm_run_mode;
+ uint8_t dm_run_version;
+ uint8_t dm_debug[4];
+} AVDOVIDmLevel255;
+
+/**
+ * Dolby Vision metadata extension block.
+ *
+ * @note sizeof(AVDOVIDmData) is not part of the public API.
+ */
+typedef struct AVDOVIDmData {
+ uint8_t level; /* [1, 255] */
+ union {
+ AVDOVIDmLevel1 l1;
+ AVDOVIDmLevel2 l2; /* may appear multiple times */
+ AVDOVIDmLevel3 l3;
+ AVDOVIDmLevel4 l4;
+ AVDOVIDmLevel5 l5;
+ AVDOVIDmLevel6 l6;
+ /* level 7 is currently unused */
+ AVDOVIDmLevel8 l8; /* may appear multiple times */
+ AVDOVIDmLevel9 l9;
+ AVDOVIDmLevel10 l10; /* may appear multiple times */
+ AVDOVIDmLevel11 l11;
+ AVDOVIDmLevel254 l254;
+ AVDOVIDmLevel255 l255;
+ };
+} AVDOVIDmData;
+
/**
* Combined struct representing a combination of header, mapping and color
* metadata, for attaching to frames as side data.
@@ -203,6 +329,10 @@ typedef struct AVDOVIMetadata {
size_t header_offset; /* AVDOVIRpuDataHeader */
size_t mapping_offset; /* AVDOVIDataMapping */
size_t color_offset; /* AVDOVIColorMetadata */
+
+ size_t ext_block_offset; /* offset to start of ext blocks array */
+ size_t ext_block_size; /* size per element */
+ int num_ext_blocks; /* number of extension blocks */
} AVDOVIMetadata;
static av_always_inline AVDOVIRpuDataHeader *
@@ -223,6 +353,19 @@ av_dovi_get_color(const AVDOVIMetadata *data)
return (AVDOVIColorMetadata *)((uint8_t *) data + data->color_offset);
}
+static av_always_inline AVDOVIDmData *
+av_dovi_get_ext(const AVDOVIMetadata *data, int index)
+{
+ return (AVDOVIDmData *)((uint8_t *) data + data->ext_block_offset +
+ data->ext_block_size * index);
+}
+
+/**
+ * Find an extension block with a given level, or NULL. In the case of
+ * multiple extension blocks, only the first is returned.
+ */
+AVDOVIDmData *av_dovi_find_level(const AVDOVIMetadata *data, uint8_t level);
+
/**
* Allocate an AVDOVIMetadata structure and initialize its
* fields to default values.
@@ -234,4 +377,15 @@ av_dovi_get_color(const AVDOVIMetadata *data)
*/
AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size);
+/**
+ * Allocate an AVDOVIMetadata with a given number of extension blocks.
+ *
+ * @param num_ext_blocks The number of extension blocks to allocate
+ * @param size If this parameter is non-NULL, the size in bytes of the
+ * allocated struct will be written here on success
+ *
+ * @return the newly allocated struct or NULL on failure
+ */
+AVDOVIMetadata *av_dovi_metadata_alloc_ext(int num_ext_blocks, size_t *size);
+
#endif /* AVUTIL_DOVI_META_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index 8a1ecd44516..f874a4fd439 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
*/
#define LIBAVUTIL_VERSION_MAJOR 59
-#define LIBAVUTIL_VERSION_MINOR 5
+#define LIBAVUTIL_VERSION_MINOR 6
#define LIBAVUTIL_VERSION_MICRO 100
#define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
--
2.44.0
_______________________________________________
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] 13+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/6] avutil/dovi_meta: add dolby vision extension blocks
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 2/6] avutil/dovi_meta: add dolby vision extension blocks Niklas Haas
@ 2024-03-23 17:58 ` Andreas Rheinhardt
2024-03-23 18:08 ` Niklas Haas
0 siblings, 1 reply; 13+ messages in thread
From: Andreas Rheinhardt @ 2024-03-23 17:58 UTC (permalink / raw)
To: ffmpeg-devel
Niklas Haas:
> From: Niklas Haas <git@haasn.dev>
>
> As well as accessors plus a function for allocating this struct with
> extension blocks,
>
> Definitions generously taken from quietvoid/dovi_tool, which is
> assembled as a collection of various patent fragments, as well as output
> by the official Dolby Vision bitstream verifier tool.
> ---
> doc/APIchanges | 5 ++
> libavutil/dovi_meta.c | 36 ++++++++--
> libavutil/dovi_meta.h | 154 ++++++++++++++++++++++++++++++++++++++++++
> libavutil/version.h | 2 +-
> 4 files changed, 190 insertions(+), 7 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 739f33501e9..8b59150dc1f 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,11 @@ The last version increases of all libraries were on 2024-03-07
>
> API changes, most recent first:
>
> +2024-03-xx - xxxxxxxxxx - lavu 59.6.100 - dovi_meta.h
> + Add AVDOVIMetadata.ext_block_{offset,size}, AVDOVIMetadata.num_ext_blocks,
> + AVDOVIDmData and AVDOVIDmLevel{1..6,8..11,254..255}, av_dovi_get_ext(),
> + av_dovi_find_level() and av_dovi_metadata_alloc_ext(),
> +
> 2024-03-xx - xxxxxxxxxx - lavu 59.5.100 - dovi_meta.h
> Add AVDOVIDataMapping.nlq_pivots.
>
> diff --git a/libavutil/dovi_meta.c b/libavutil/dovi_meta.c
> index 9c50da561ed..17e18bf95f9 100644
> --- a/libavutil/dovi_meta.c
> +++ b/libavutil/dovi_meta.c
> @@ -18,6 +18,8 @@
> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> */
>
> +#include <string.h>
> +
> #include "dovi_meta.h"
> #include "mem.h"
>
> @@ -39,22 +41,44 @@ typedef struct AVDOVIMetadataInternal {
> AVDOVIRpuDataHeader header;
> AVDOVIDataMapping mapping;
> AVDOVIColorMetadata color;
> + AVDOVIDmData ext_blocks[];
> } AVDOVIMetadataInternal;
>
> -AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size)
> +AVDOVIMetadata *av_dovi_metadata_alloc_ext(int num_ext_blocks, size_t *size)
> {
> - AVDOVIMetadataInternal *dovi = av_mallocz(sizeof(AVDOVIMetadataInternal));
> + const size_t alloc_size = sizeof(AVDOVIMetadataInternal) +
> + num_ext_blocks * sizeof(AVDOVIDmData);
> +
> + AVDOVIMetadataInternal *dovi = av_mallocz(alloc_size);
> if (!dovi)
> return NULL;
>
> if (size)
> - *size = sizeof(*dovi);
> + *size = alloc_size;
>
> dovi->metadata = (struct AVDOVIMetadata) {
> - .header_offset = offsetof(AVDOVIMetadataInternal, header),
> - .mapping_offset = offsetof(AVDOVIMetadataInternal, mapping),
> - .color_offset = offsetof(AVDOVIMetadataInternal, color),
> + .header_offset = offsetof(AVDOVIMetadataInternal, header),
> + .mapping_offset = offsetof(AVDOVIMetadataInternal, mapping),
> + .color_offset = offsetof(AVDOVIMetadataInternal, color),
> + .ext_block_offset = offsetof(AVDOVIMetadataInternal, ext_blocks),
> + .ext_block_size = sizeof(AVDOVIDmData),
> };
>
> return &dovi->metadata;
> }
> +
> +AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size)
> +{
> + return av_dovi_metadata_alloc_ext(0, size);
> +}
> +
> +AVDOVIDmData *av_dovi_find_level(const AVDOVIMetadata *data, uint8_t level)
> +{
> + for (int i = 0; i < data->num_ext_blocks; i++) {
> + AVDOVIDmData *ext = av_dovi_get_ext(data, i);
> + if (ext->level == level)
> + return ext;
> + }
> +
> + return NULL;
> +}
> diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
> index 6afc7b055a7..e77d9853f67 100644
> --- a/libavutil/dovi_meta.h
> +++ b/libavutil/dovi_meta.h
> @@ -29,7 +29,9 @@
>
> #include <stdint.h>
> #include <stddef.h>
> +
> #include "rational.h"
> +#include "csp.h"
>
> /*
> * DOVI configuration
> @@ -187,6 +189,130 @@ typedef struct AVDOVIColorMetadata {
> uint16_t source_diagonal;
> } AVDOVIColorMetadata;
>
> +typedef struct AVDOVIDmLevel1 {
> + /* Per-frame brightness metadata */
> + uint16_t min_pq;
> + uint16_t max_pq;
> + uint16_t avg_pq;
> +} AVDOVIDmLevel1;
> +
> +typedef struct AVDOVIDmLevel2 {
> + /* Usually derived from level 8 (at different levels) */
> + uint16_t target_max_pq;
> + uint16_t trim_slope;
> + uint16_t trim_offset;
> + uint16_t trim_power;
> + uint16_t trim_chroma_weight;
> + uint16_t trim_saturation_gain;
> + int16_t ms_weight;
> +} AVDOVIDmLevel2;
> +
> +typedef struct AVDOVIDmLevel3 {
> + uint16_t min_pq_offset;
> + uint16_t max_pq_offset;
> + uint16_t avg_pq_offset;
> +} AVDOVIDmLevel3;
> +
> +typedef struct AVDOVIDmLevel4 {
> + uint16_t anchor_pq;
> + uint16_t anchor_power;
> +} AVDOVIDmLevel4;
> +
> +typedef struct AVDOVIDmLevel5 {
> + /* Active area definition */
> + uint16_t left_offset;
> + uint16_t right_offset;
> + uint16_t top_offset;
> + uint16_t bottom_offset;
> +} AVDOVIDmLevel5;
> +
> +typedef struct AVDOVIDmLevel6 {
> + /* Static HDR10 metadata */
> + uint16_t max_luminance;
> + uint16_t min_luminance;
> + uint16_t max_cll;
> + uint16_t max_fall;
> +} AVDOVIDmLevel6;
> +
> +typedef struct AVDOVIDmLevel8 {
> + /* Extended version of level 2 */
> + uint8_t target_display_index;
> + uint16_t trim_slope;
> + uint16_t trim_offset;
> + uint16_t trim_power;
> + uint16_t trim_chroma_weight;
> + uint16_t trim_saturation_gain;
> + uint16_t ms_weight;
> + uint16_t target_mid_contrast;
> + uint16_t clip_trim;
> + uint8_t saturation_vector_field[6];
> + uint8_t hue_vector_field[6];
> +} AVDOVIDmLevel8;
> +
> +typedef struct AVDOVIDmLevel9 {
> + /* Source display characteristics */
> + uint8_t source_primary_index;
> + AVColorPrimariesDesc source_display_primaries;
> +} AVDOVIDmLevel9;
> +
> +typedef struct AVDOVIDmLevel10 {
> + /* Target display characteristics */
> + uint8_t target_display_index;
> + uint16_t target_max_pq;
> + uint16_t target_min_pq;
> + uint8_t target_primary_index;
> + AVColorPrimariesDesc target_display_primaries;
> +} AVDOVIDmLevel10;
> +
> +typedef struct AVDOVIDmLevel11 {
> + uint8_t content_type;
> + uint8_t whitepoint;
> + uint8_t reference_mode_flag;
> + uint8_t sharpness;
> + uint8_t noise_reduction;
> + uint8_t mpeg_noise_reduction;
> + uint8_t frame_rate_conversion;
> + uint8_t brightness;
> + uint8_t color;
> +} AVDOVIDmLevel11;
> +
> +typedef struct AVDOVIDmLevel254 {
> + /* DMv2 info block, always present in samples with DMv2 metadata */
> + uint8_t dm_mode;
> + uint8_t dm_version_index;
> +} AVDOVIDmLevel254;
> +
> +typedef struct AVDOVIDmLevel255 {
> + /* Debug block, not really used in samples */
> + uint8_t dm_run_mode;
> + uint8_t dm_run_version;
> + uint8_t dm_debug[4];
> +} AVDOVIDmLevel255;
> +
> +/**
> + * Dolby Vision metadata extension block.
> + *
> + * @note sizeof(AVDOVIDmData) is not part of the public API.
> + */
> +typedef struct AVDOVIDmData {
> + uint8_t level; /* [1, 255] */
> + union {
> + AVDOVIDmLevel1 l1;
> + AVDOVIDmLevel2 l2; /* may appear multiple times */
> + AVDOVIDmLevel3 l3;
> + AVDOVIDmLevel4 l4;
> + AVDOVIDmLevel5 l5;
> + AVDOVIDmLevel6 l6;
> + /* level 7 is currently unused */
> + AVDOVIDmLevel8 l8; /* may appear multiple times */
> + AVDOVIDmLevel9 l9;
> + AVDOVIDmLevel10 l10; /* may appear multiple times */
> + AVDOVIDmLevel11 l11;
> + AVDOVIDmLevel254 l254;
> + AVDOVIDmLevel255 l255;
> + };
Unnamed unions are C11
> +} AVDOVIDmData;
> +
> /**
> * Combined struct representing a combination of header, mapping and color
> * metadata, for attaching to frames as side data.
> @@ -203,6 +329,10 @@ typedef struct AVDOVIMetadata {
> size_t header_offset; /* AVDOVIRpuDataHeader */
> size_t mapping_offset; /* AVDOVIDataMapping */
> size_t color_offset; /* AVDOVIColorMetadata */
> +
> + size_t ext_block_offset; /* offset to start of ext blocks array */
> + size_t ext_block_size; /* size per element */
> + int num_ext_blocks; /* number of extension blocks */
> } AVDOVIMetadata;
>
> static av_always_inline AVDOVIRpuDataHeader *
> @@ -223,6 +353,19 @@ av_dovi_get_color(const AVDOVIMetadata *data)
> return (AVDOVIColorMetadata *)((uint8_t *) data + data->color_offset);
> }
>
> +static av_always_inline AVDOVIDmData *
> +av_dovi_get_ext(const AVDOVIMetadata *data, int index)
> +{
> + return (AVDOVIDmData *)((uint8_t *) data + data->ext_block_offset +
> + data->ext_block_size * index);
This is not const-correct.
> +}
> +
> +/**
> + * Find an extension block with a given level, or NULL. In the case of
> + * multiple extension blocks, only the first is returned.
> + */
> +AVDOVIDmData *av_dovi_find_level(const AVDOVIMetadata *data, uint8_t level);
> +
> /**
> * Allocate an AVDOVIMetadata structure and initialize its
> * fields to default values.
> @@ -234,4 +377,15 @@ av_dovi_get_color(const AVDOVIMetadata *data)
> */
> AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size);
>
> +/**
> + * Allocate an AVDOVIMetadata with a given number of extension blocks.
> + *
> + * @param num_ext_blocks The number of extension blocks to allocate
> + * @param size If this parameter is non-NULL, the size in bytes of the
> + * allocated struct will be written here on success
> + *
> + * @return the newly allocated struct or NULL on failure
> + */
> +AVDOVIMetadata *av_dovi_metadata_alloc_ext(int num_ext_blocks, size_t *size);
> +
Not another boilerplate allocator.
> #endif /* AVUTIL_DOVI_META_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 8a1ecd44516..f874a4fd439 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 59
> -#define LIBAVUTIL_VERSION_MINOR 5
> +#define LIBAVUTIL_VERSION_MINOR 6
> #define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
_______________________________________________
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] 13+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/6] avutil/dovi_meta: add dolby vision extension blocks
2024-03-23 17:58 ` Andreas Rheinhardt
@ 2024-03-23 18:08 ` Niklas Haas
2024-03-23 18:45 ` Niklas Haas
0 siblings, 1 reply; 13+ messages in thread
From: Niklas Haas @ 2024-03-23 18:08 UTC (permalink / raw)
To: ffmpeg-devel
On Sat, 23 Mar 2024 18:58:51 +0100 Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
> Niklas Haas:
> > From: Niklas Haas <git@haasn.dev>
> >
> > As well as accessors plus a function for allocating this struct with
> > extension blocks,
> >
> > Definitions generously taken from quietvoid/dovi_tool, which is
> > assembled as a collection of various patent fragments, as well as output
> > by the official Dolby Vision bitstream verifier tool.
> > ---
> > doc/APIchanges | 5 ++
> > libavutil/dovi_meta.c | 36 ++++++++--
> > libavutil/dovi_meta.h | 154 ++++++++++++++++++++++++++++++++++++++++++
> > libavutil/version.h | 2 +-
> > 4 files changed, 190 insertions(+), 7 deletions(-)
> >
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 739f33501e9..8b59150dc1f 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -2,6 +2,11 @@ The last version increases of all libraries were on 2024-03-07
> >
> > API changes, most recent first:
> >
> > +2024-03-xx - xxxxxxxxxx - lavu 59.6.100 - dovi_meta.h
> > + Add AVDOVIMetadata.ext_block_{offset,size}, AVDOVIMetadata.num_ext_blocks,
> > + AVDOVIDmData and AVDOVIDmLevel{1..6,8..11,254..255}, av_dovi_get_ext(),
> > + av_dovi_find_level() and av_dovi_metadata_alloc_ext(),
> > +
> > 2024-03-xx - xxxxxxxxxx - lavu 59.5.100 - dovi_meta.h
> > Add AVDOVIDataMapping.nlq_pivots.
> >
> > diff --git a/libavutil/dovi_meta.c b/libavutil/dovi_meta.c
> > index 9c50da561ed..17e18bf95f9 100644
> > --- a/libavutil/dovi_meta.c
> > +++ b/libavutil/dovi_meta.c
> > @@ -18,6 +18,8 @@
> > * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > */
> >
> > +#include <string.h>
> > +
> > #include "dovi_meta.h"
> > #include "mem.h"
> >
> > @@ -39,22 +41,44 @@ typedef struct AVDOVIMetadataInternal {
> > AVDOVIRpuDataHeader header;
> > AVDOVIDataMapping mapping;
> > AVDOVIColorMetadata color;
> > + AVDOVIDmData ext_blocks[];
> > } AVDOVIMetadataInternal;
> >
> > -AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size)
> > +AVDOVIMetadata *av_dovi_metadata_alloc_ext(int num_ext_blocks, size_t *size)
> > {
> > - AVDOVIMetadataInternal *dovi = av_mallocz(sizeof(AVDOVIMetadataInternal));
> > + const size_t alloc_size = sizeof(AVDOVIMetadataInternal) +
> > + num_ext_blocks * sizeof(AVDOVIDmData);
> > +
> > + AVDOVIMetadataInternal *dovi = av_mallocz(alloc_size);
> > if (!dovi)
> > return NULL;
> >
> > if (size)
> > - *size = sizeof(*dovi);
> > + *size = alloc_size;
> >
> > dovi->metadata = (struct AVDOVIMetadata) {
> > - .header_offset = offsetof(AVDOVIMetadataInternal, header),
> > - .mapping_offset = offsetof(AVDOVIMetadataInternal, mapping),
> > - .color_offset = offsetof(AVDOVIMetadataInternal, color),
> > + .header_offset = offsetof(AVDOVIMetadataInternal, header),
> > + .mapping_offset = offsetof(AVDOVIMetadataInternal, mapping),
> > + .color_offset = offsetof(AVDOVIMetadataInternal, color),
> > + .ext_block_offset = offsetof(AVDOVIMetadataInternal, ext_blocks),
> > + .ext_block_size = sizeof(AVDOVIDmData),
> > };
> >
> > return &dovi->metadata;
> > }
> > +
> > +AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size)
> > +{
> > + return av_dovi_metadata_alloc_ext(0, size);
> > +}
> > +
> > +AVDOVIDmData *av_dovi_find_level(const AVDOVIMetadata *data, uint8_t level)
> > +{
> > + for (int i = 0; i < data->num_ext_blocks; i++) {
> > + AVDOVIDmData *ext = av_dovi_get_ext(data, i);
> > + if (ext->level == level)
> > + return ext;
> > + }
> > +
> > + return NULL;
> > +}
> > diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
> > index 6afc7b055a7..e77d9853f67 100644
> > --- a/libavutil/dovi_meta.h
> > +++ b/libavutil/dovi_meta.h
> > @@ -29,7 +29,9 @@
> >
> > #include <stdint.h>
> > #include <stddef.h>
> > +
> > #include "rational.h"
> > +#include "csp.h"
> >
> > /*
> > * DOVI configuration
> > @@ -187,6 +189,130 @@ typedef struct AVDOVIColorMetadata {
> > uint16_t source_diagonal;
> > } AVDOVIColorMetadata;
> >
> > +typedef struct AVDOVIDmLevel1 {
> > + /* Per-frame brightness metadata */
> > + uint16_t min_pq;
> > + uint16_t max_pq;
> > + uint16_t avg_pq;
> > +} AVDOVIDmLevel1;
> > +
> > +typedef struct AVDOVIDmLevel2 {
> > + /* Usually derived from level 8 (at different levels) */
> > + uint16_t target_max_pq;
> > + uint16_t trim_slope;
> > + uint16_t trim_offset;
> > + uint16_t trim_power;
> > + uint16_t trim_chroma_weight;
> > + uint16_t trim_saturation_gain;
> > + int16_t ms_weight;
> > +} AVDOVIDmLevel2;
> > +
> > +typedef struct AVDOVIDmLevel3 {
> > + uint16_t min_pq_offset;
> > + uint16_t max_pq_offset;
> > + uint16_t avg_pq_offset;
> > +} AVDOVIDmLevel3;
> > +
> > +typedef struct AVDOVIDmLevel4 {
> > + uint16_t anchor_pq;
> > + uint16_t anchor_power;
> > +} AVDOVIDmLevel4;
> > +
> > +typedef struct AVDOVIDmLevel5 {
> > + /* Active area definition */
> > + uint16_t left_offset;
> > + uint16_t right_offset;
> > + uint16_t top_offset;
> > + uint16_t bottom_offset;
> > +} AVDOVIDmLevel5;
> > +
> > +typedef struct AVDOVIDmLevel6 {
> > + /* Static HDR10 metadata */
> > + uint16_t max_luminance;
> > + uint16_t min_luminance;
> > + uint16_t max_cll;
> > + uint16_t max_fall;
> > +} AVDOVIDmLevel6;
> > +
> > +typedef struct AVDOVIDmLevel8 {
> > + /* Extended version of level 2 */
> > + uint8_t target_display_index;
> > + uint16_t trim_slope;
> > + uint16_t trim_offset;
> > + uint16_t trim_power;
> > + uint16_t trim_chroma_weight;
> > + uint16_t trim_saturation_gain;
> > + uint16_t ms_weight;
> > + uint16_t target_mid_contrast;
> > + uint16_t clip_trim;
> > + uint8_t saturation_vector_field[6];
> > + uint8_t hue_vector_field[6];
> > +} AVDOVIDmLevel8;
> > +
> > +typedef struct AVDOVIDmLevel9 {
> > + /* Source display characteristics */
> > + uint8_t source_primary_index;
> > + AVColorPrimariesDesc source_display_primaries;
> > +} AVDOVIDmLevel9;
> > +
> > +typedef struct AVDOVIDmLevel10 {
> > + /* Target display characteristics */
> > + uint8_t target_display_index;
> > + uint16_t target_max_pq;
> > + uint16_t target_min_pq;
> > + uint8_t target_primary_index;
> > + AVColorPrimariesDesc target_display_primaries;
> > +} AVDOVIDmLevel10;
> > +
> > +typedef struct AVDOVIDmLevel11 {
> > + uint8_t content_type;
> > + uint8_t whitepoint;
> > + uint8_t reference_mode_flag;
> > + uint8_t sharpness;
> > + uint8_t noise_reduction;
> > + uint8_t mpeg_noise_reduction;
> > + uint8_t frame_rate_conversion;
> > + uint8_t brightness;
> > + uint8_t color;
> > +} AVDOVIDmLevel11;
> > +
> > +typedef struct AVDOVIDmLevel254 {
> > + /* DMv2 info block, always present in samples with DMv2 metadata */
> > + uint8_t dm_mode;
> > + uint8_t dm_version_index;
> > +} AVDOVIDmLevel254;
> > +
> > +typedef struct AVDOVIDmLevel255 {
> > + /* Debug block, not really used in samples */
> > + uint8_t dm_run_mode;
> > + uint8_t dm_run_version;
> > + uint8_t dm_debug[4];
> > +} AVDOVIDmLevel255;
> > +
> > +/**
> > + * Dolby Vision metadata extension block.
> > + *
> > + * @note sizeof(AVDOVIDmData) is not part of the public API.
> > + */
> > +typedef struct AVDOVIDmData {
> > + uint8_t level; /* [1, 255] */
> > + union {
> > + AVDOVIDmLevel1 l1;
> > + AVDOVIDmLevel2 l2; /* may appear multiple times */
> > + AVDOVIDmLevel3 l3;
> > + AVDOVIDmLevel4 l4;
> > + AVDOVIDmLevel5 l5;
> > + AVDOVIDmLevel6 l6;
> > + /* level 7 is currently unused */
> > + AVDOVIDmLevel8 l8; /* may appear multiple times */
> > + AVDOVIDmLevel9 l9;
> > + AVDOVIDmLevel10 l10; /* may appear multiple times */
> > + AVDOVIDmLevel11 l11;
> > + AVDOVIDmLevel254 l254;
> > + AVDOVIDmLevel255 l255;
> > + };
>
> Unnamed unions are C11
I thought we support C11 now?
>
> > +} AVDOVIDmData;
> > +
> > /**
> > * Combined struct representing a combination of header, mapping and color
> > * metadata, for attaching to frames as side data.
> > @@ -203,6 +329,10 @@ typedef struct AVDOVIMetadata {
> > size_t header_offset; /* AVDOVIRpuDataHeader */
> > size_t mapping_offset; /* AVDOVIDataMapping */
> > size_t color_offset; /* AVDOVIColorMetadata */
> > +
> > + size_t ext_block_offset; /* offset to start of ext blocks array */
> > + size_t ext_block_size; /* size per element */
> > + int num_ext_blocks; /* number of extension blocks */
> > } AVDOVIMetadata;
> >
> > static av_always_inline AVDOVIRpuDataHeader *
> > @@ -223,6 +353,19 @@ av_dovi_get_color(const AVDOVIMetadata *data)
> > return (AVDOVIColorMetadata *)((uint8_t *) data + data->color_offset);
> > }
> >
> > +static av_always_inline AVDOVIDmData *
> > +av_dovi_get_ext(const AVDOVIMetadata *data, int index)
> > +{
> > + return (AVDOVIDmData *)((uint8_t *) data + data->ext_block_offset +
> > + data->ext_block_size * index);
>
> This is not const-correct.
It follows the convention of the functions immediately above it. What do
you suggest? Always returning `const` here is awkward for users who want
to write to it, but never returning `const` is awkward for users who
want to read from it.
C really isn't the right tool for the job here..
>
> > +}
> > +
> > +/**
> > + * Find an extension block with a given level, or NULL. In the case of
> > + * multiple extension blocks, only the first is returned.
> > + */
> > +AVDOVIDmData *av_dovi_find_level(const AVDOVIMetadata *data, uint8_t level);
> > +
> > /**
> > * Allocate an AVDOVIMetadata structure and initialize its
> > * fields to default values.
> > @@ -234,4 +377,15 @@ av_dovi_get_color(const AVDOVIMetadata *data)
> > */
> > AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size);
> >
> > +/**
> > + * Allocate an AVDOVIMetadata with a given number of extension blocks.
> > + *
> > + * @param num_ext_blocks The number of extension blocks to allocate
> > + * @param size If this parameter is non-NULL, the size in bytes of the
> > + * allocated struct will be written here on success
> > + *
> > + * @return the newly allocated struct or NULL on failure
> > + */
> > +AVDOVIMetadata *av_dovi_metadata_alloc_ext(int num_ext_blocks, size_t *size);
> > +
>
> Not another boilerplate allocator.
What do you suggest?
>
> > #endif /* AVUTIL_DOVI_META_H */
> > diff --git a/libavutil/version.h b/libavutil/version.h
> > index 8a1ecd44516..f874a4fd439 100644
> > --- a/libavutil/version.h
> > +++ b/libavutil/version.h
> > @@ -79,7 +79,7 @@
> > */
> >
> > #define LIBAVUTIL_VERSION_MAJOR 59
> > -#define LIBAVUTIL_VERSION_MINOR 5
> > +#define LIBAVUTIL_VERSION_MINOR 6
> > #define LIBAVUTIL_VERSION_MICRO 100
> >
> > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>
> _______________________________________________
> 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".
_______________________________________________
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] 13+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/6] avutil/dovi_meta: add dolby vision extension blocks
2024-03-23 18:08 ` Niklas Haas
@ 2024-03-23 18:45 ` Niklas Haas
0 siblings, 0 replies; 13+ messages in thread
From: Niklas Haas @ 2024-03-23 18:45 UTC (permalink / raw)
To: ffmpeg-devel
On Sat, 23 Mar 2024 19:08:04 +0100 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> On Sat, 23 Mar 2024 18:58:51 +0100 Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
> > Niklas Haas:
> > > From: Niklas Haas <git@haasn.dev>
> > >
> > > As well as accessors plus a function for allocating this struct with
> > > extension blocks,
> > >
> > > Definitions generously taken from quietvoid/dovi_tool, which is
> > > assembled as a collection of various patent fragments, as well as output
> > > by the official Dolby Vision bitstream verifier tool.
> > > ---
> > > doc/APIchanges | 5 ++
> > > libavutil/dovi_meta.c | 36 ++++++++--
> > > libavutil/dovi_meta.h | 154 ++++++++++++++++++++++++++++++++++++++++++
> > > libavutil/version.h | 2 +-
> > > 4 files changed, 190 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/doc/APIchanges b/doc/APIchanges
> > > index 739f33501e9..8b59150dc1f 100644
> > > --- a/doc/APIchanges
> > > +++ b/doc/APIchanges
> > > @@ -2,6 +2,11 @@ The last version increases of all libraries were on 2024-03-07
> > >
> > > API changes, most recent first:
> > >
> > > +2024-03-xx - xxxxxxxxxx - lavu 59.6.100 - dovi_meta.h
> > > + Add AVDOVIMetadata.ext_block_{offset,size}, AVDOVIMetadata.num_ext_blocks,
> > > + AVDOVIDmData and AVDOVIDmLevel{1..6,8..11,254..255}, av_dovi_get_ext(),
> > > + av_dovi_find_level() and av_dovi_metadata_alloc_ext(),
> > > +
> > > 2024-03-xx - xxxxxxxxxx - lavu 59.5.100 - dovi_meta.h
> > > Add AVDOVIDataMapping.nlq_pivots.
> > >
> > > diff --git a/libavutil/dovi_meta.c b/libavutil/dovi_meta.c
> > > index 9c50da561ed..17e18bf95f9 100644
> > > --- a/libavutil/dovi_meta.c
> > > +++ b/libavutil/dovi_meta.c
> > > @@ -18,6 +18,8 @@
> > > * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > > */
> > >
> > > +#include <string.h>
> > > +
> > > #include "dovi_meta.h"
> > > #include "mem.h"
> > >
> > > @@ -39,22 +41,44 @@ typedef struct AVDOVIMetadataInternal {
> > > AVDOVIRpuDataHeader header;
> > > AVDOVIDataMapping mapping;
> > > AVDOVIColorMetadata color;
> > > + AVDOVIDmData ext_blocks[];
> > > } AVDOVIMetadataInternal;
> > >
> > > -AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size)
> > > +AVDOVIMetadata *av_dovi_metadata_alloc_ext(int num_ext_blocks, size_t *size)
> > > {
> > > - AVDOVIMetadataInternal *dovi = av_mallocz(sizeof(AVDOVIMetadataInternal));
> > > + const size_t alloc_size = sizeof(AVDOVIMetadataInternal) +
> > > + num_ext_blocks * sizeof(AVDOVIDmData);
> > > +
> > > + AVDOVIMetadataInternal *dovi = av_mallocz(alloc_size);
> > > if (!dovi)
> > > return NULL;
> > >
> > > if (size)
> > > - *size = sizeof(*dovi);
> > > + *size = alloc_size;
> > >
> > > dovi->metadata = (struct AVDOVIMetadata) {
> > > - .header_offset = offsetof(AVDOVIMetadataInternal, header),
> > > - .mapping_offset = offsetof(AVDOVIMetadataInternal, mapping),
> > > - .color_offset = offsetof(AVDOVIMetadataInternal, color),
> > > + .header_offset = offsetof(AVDOVIMetadataInternal, header),
> > > + .mapping_offset = offsetof(AVDOVIMetadataInternal, mapping),
> > > + .color_offset = offsetof(AVDOVIMetadataInternal, color),
> > > + .ext_block_offset = offsetof(AVDOVIMetadataInternal, ext_blocks),
> > > + .ext_block_size = sizeof(AVDOVIDmData),
> > > };
> > >
> > > return &dovi->metadata;
> > > }
> > > +
> > > +AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size)
> > > +{
> > > + return av_dovi_metadata_alloc_ext(0, size);
> > > +}
> > > +
> > > +AVDOVIDmData *av_dovi_find_level(const AVDOVIMetadata *data, uint8_t level)
> > > +{
> > > + for (int i = 0; i < data->num_ext_blocks; i++) {
> > > + AVDOVIDmData *ext = av_dovi_get_ext(data, i);
> > > + if (ext->level == level)
> > > + return ext;
> > > + }
> > > +
> > > + return NULL;
> > > +}
> > > diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
> > > index 6afc7b055a7..e77d9853f67 100644
> > > --- a/libavutil/dovi_meta.h
> > > +++ b/libavutil/dovi_meta.h
> > > @@ -29,7 +29,9 @@
> > >
> > > #include <stdint.h>
> > > #include <stddef.h>
> > > +
> > > #include "rational.h"
> > > +#include "csp.h"
> > >
> > > /*
> > > * DOVI configuration
> > > @@ -187,6 +189,130 @@ typedef struct AVDOVIColorMetadata {
> > > uint16_t source_diagonal;
> > > } AVDOVIColorMetadata;
> > >
> > > +typedef struct AVDOVIDmLevel1 {
> > > + /* Per-frame brightness metadata */
> > > + uint16_t min_pq;
> > > + uint16_t max_pq;
> > > + uint16_t avg_pq;
> > > +} AVDOVIDmLevel1;
> > > +
> > > +typedef struct AVDOVIDmLevel2 {
> > > + /* Usually derived from level 8 (at different levels) */
> > > + uint16_t target_max_pq;
> > > + uint16_t trim_slope;
> > > + uint16_t trim_offset;
> > > + uint16_t trim_power;
> > > + uint16_t trim_chroma_weight;
> > > + uint16_t trim_saturation_gain;
> > > + int16_t ms_weight;
> > > +} AVDOVIDmLevel2;
> > > +
> > > +typedef struct AVDOVIDmLevel3 {
> > > + uint16_t min_pq_offset;
> > > + uint16_t max_pq_offset;
> > > + uint16_t avg_pq_offset;
> > > +} AVDOVIDmLevel3;
> > > +
> > > +typedef struct AVDOVIDmLevel4 {
> > > + uint16_t anchor_pq;
> > > + uint16_t anchor_power;
> > > +} AVDOVIDmLevel4;
> > > +
> > > +typedef struct AVDOVIDmLevel5 {
> > > + /* Active area definition */
> > > + uint16_t left_offset;
> > > + uint16_t right_offset;
> > > + uint16_t top_offset;
> > > + uint16_t bottom_offset;
> > > +} AVDOVIDmLevel5;
> > > +
> > > +typedef struct AVDOVIDmLevel6 {
> > > + /* Static HDR10 metadata */
> > > + uint16_t max_luminance;
> > > + uint16_t min_luminance;
> > > + uint16_t max_cll;
> > > + uint16_t max_fall;
> > > +} AVDOVIDmLevel6;
> > > +
> > > +typedef struct AVDOVIDmLevel8 {
> > > + /* Extended version of level 2 */
> > > + uint8_t target_display_index;
> > > + uint16_t trim_slope;
> > > + uint16_t trim_offset;
> > > + uint16_t trim_power;
> > > + uint16_t trim_chroma_weight;
> > > + uint16_t trim_saturation_gain;
> > > + uint16_t ms_weight;
> > > + uint16_t target_mid_contrast;
> > > + uint16_t clip_trim;
> > > + uint8_t saturation_vector_field[6];
> > > + uint8_t hue_vector_field[6];
> > > +} AVDOVIDmLevel8;
> > > +
> > > +typedef struct AVDOVIDmLevel9 {
> > > + /* Source display characteristics */
> > > + uint8_t source_primary_index;
> > > + AVColorPrimariesDesc source_display_primaries;
> > > +} AVDOVIDmLevel9;
> > > +
> > > +typedef struct AVDOVIDmLevel10 {
> > > + /* Target display characteristics */
> > > + uint8_t target_display_index;
> > > + uint16_t target_max_pq;
> > > + uint16_t target_min_pq;
> > > + uint8_t target_primary_index;
> > > + AVColorPrimariesDesc target_display_primaries;
> > > +} AVDOVIDmLevel10;
> > > +
> > > +typedef struct AVDOVIDmLevel11 {
> > > + uint8_t content_type;
> > > + uint8_t whitepoint;
> > > + uint8_t reference_mode_flag;
> > > + uint8_t sharpness;
> > > + uint8_t noise_reduction;
> > > + uint8_t mpeg_noise_reduction;
> > > + uint8_t frame_rate_conversion;
> > > + uint8_t brightness;
> > > + uint8_t color;
> > > +} AVDOVIDmLevel11;
> > > +
> > > +typedef struct AVDOVIDmLevel254 {
> > > + /* DMv2 info block, always present in samples with DMv2 metadata */
> > > + uint8_t dm_mode;
> > > + uint8_t dm_version_index;
> > > +} AVDOVIDmLevel254;
> > > +
> > > +typedef struct AVDOVIDmLevel255 {
> > > + /* Debug block, not really used in samples */
> > > + uint8_t dm_run_mode;
> > > + uint8_t dm_run_version;
> > > + uint8_t dm_debug[4];
> > > +} AVDOVIDmLevel255;
> > > +
> > > +/**
> > > + * Dolby Vision metadata extension block.
> > > + *
> > > + * @note sizeof(AVDOVIDmData) is not part of the public API.
> > > + */
> > > +typedef struct AVDOVIDmData {
> > > + uint8_t level; /* [1, 255] */
> > > + union {
> > > + AVDOVIDmLevel1 l1;
> > > + AVDOVIDmLevel2 l2; /* may appear multiple times */
> > > + AVDOVIDmLevel3 l3;
> > > + AVDOVIDmLevel4 l4;
> > > + AVDOVIDmLevel5 l5;
> > > + AVDOVIDmLevel6 l6;
> > > + /* level 7 is currently unused */
> > > + AVDOVIDmLevel8 l8; /* may appear multiple times */
> > > + AVDOVIDmLevel9 l9;
> > > + AVDOVIDmLevel10 l10; /* may appear multiple times */
> > > + AVDOVIDmLevel11 l11;
> > > + AVDOVIDmLevel254 l254;
> > > + AVDOVIDmLevel255 l255;
> > > + };
> >
> > Unnamed unions are C11
>
> I thought we support C11 now?
>
> >
> > > +} AVDOVIDmData;
> > > +
> > > /**
> > > * Combined struct representing a combination of header, mapping and color
> > > * metadata, for attaching to frames as side data.
> > > @@ -203,6 +329,10 @@ typedef struct AVDOVIMetadata {
> > > size_t header_offset; /* AVDOVIRpuDataHeader */
> > > size_t mapping_offset; /* AVDOVIDataMapping */
> > > size_t color_offset; /* AVDOVIColorMetadata */
> > > +
> > > + size_t ext_block_offset; /* offset to start of ext blocks array */
> > > + size_t ext_block_size; /* size per element */
> > > + int num_ext_blocks; /* number of extension blocks */
> > > } AVDOVIMetadata;
> > >
> > > static av_always_inline AVDOVIRpuDataHeader *
> > > @@ -223,6 +353,19 @@ av_dovi_get_color(const AVDOVIMetadata *data)
> > > return (AVDOVIColorMetadata *)((uint8_t *) data + data->color_offset);
> > > }
> > >
> > > +static av_always_inline AVDOVIDmData *
> > > +av_dovi_get_ext(const AVDOVIMetadata *data, int index)
> > > +{
> > > + return (AVDOVIDmData *)((uint8_t *) data + data->ext_block_offset +
> > > + data->ext_block_size * index);
> >
> > This is not const-correct.
>
> It follows the convention of the functions immediately above it. What do
> you suggest? Always returning `const` here is awkward for users who want
> to write to it, but never returning `const` is awkward for users who
> want to read from it.
>
> C really isn't the right tool for the job here..
>
> >
> > > +}
> > > +
> > > +/**
> > > + * Find an extension block with a given level, or NULL. In the case of
> > > + * multiple extension blocks, only the first is returned.
> > > + */
> > > +AVDOVIDmData *av_dovi_find_level(const AVDOVIMetadata *data, uint8_t level);
> > > +
> > > /**
> > > * Allocate an AVDOVIMetadata structure and initialize its
> > > * fields to default values.
> > > @@ -234,4 +377,15 @@ av_dovi_get_color(const AVDOVIMetadata *data)
> > > */
> > > AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size);
> > >
> > > +/**
> > > + * Allocate an AVDOVIMetadata with a given number of extension blocks.
> > > + *
> > > + * @param num_ext_blocks The number of extension blocks to allocate
> > > + * @param size If this parameter is non-NULL, the size in bytes of the
> > > + * allocated struct will be written here on success
> > > + *
> > > + * @return the newly allocated struct or NULL on failure
> > > + */
> > > +AVDOVIMetadata *av_dovi_metadata_alloc_ext(int num_ext_blocks, size_t *size);
> > > +
> >
> > Not another boilerplate allocator.
>
> What do you suggest?
Update: I decided to drop dynamic allocation of this because it turns
out that we're constrained to a maximum of ~32 extension blocks anyway,
and the existing struct size (~5 kB) is already substantially larger
than the size it would take to just always allocate space for 32
extension blocks.
>
> >
> > > #endif /* AVUTIL_DOVI_META_H */
> > > diff --git a/libavutil/version.h b/libavutil/version.h
> > > index 8a1ecd44516..f874a4fd439 100644
> > > --- a/libavutil/version.h
> > > +++ b/libavutil/version.h
> > > @@ -79,7 +79,7 @@
> > > */
> > >
> > > #define LIBAVUTIL_VERSION_MAJOR 59
> > > -#define LIBAVUTIL_VERSION_MINOR 5
> > > +#define LIBAVUTIL_VERSION_MINOR 6
> > > #define LIBAVUTIL_VERSION_MICRO 100
> > >
> > > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> >
> > _______________________________________________
> > 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".
_______________________________________________
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] 13+ messages in thread
* [FFmpeg-devel] [PATCH 3/6] avcodec/dovi_rpu: strip container in separate step
2024-03-23 17:37 [FFmpeg-devel] [PATCH 1/6] avutil/dovi_meta: add AVDOVIDataMapping.nlq_pivots Niklas Haas
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 2/6] avutil/dovi_meta: add dolby vision extension blocks Niklas Haas
@ 2024-03-23 17:37 ` Niklas Haas
2024-03-23 18:02 ` Andreas Rheinhardt
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 4/6] avcodec/dovi_rpu: verify RPU data CRC32 Niklas Haas
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Niklas Haas @ 2024-03-23 17:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
This ensures that `gb` in the following section is fully byte-aligned,
points at the start of the actual RPU, and ends on the CRC terminator.
This is important for both calculation of the CRC, as well as dovi
extension block parsing (which aligns to byte boundaries in various
places).
---
libavcodec/dovi_rpu.c | 48 +++++++++++++++++++++++++++++++++++--------
libavcodec/dovi_rpu.h | 2 ++
2 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index c84a942f476..24a1cdf39a8 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -23,6 +23,7 @@
#include "libavutil/buffer.h"
+#include "avcodec.h"
#include "dovi_rpu.h"
#include "golomb.h"
#include "get_bits.h"
@@ -45,6 +46,7 @@ void ff_dovi_ctx_unref(DOVIContext *s)
{
for (int i = 0; i < FF_ARRAY_ELEMS(s->vdr); i++)
ff_refstruct_unref(&s->vdr[i]);
+ av_free(s->rpu_buf);
*s = (DOVIContext) {
.logctx = s->logctx,
@@ -202,17 +204,17 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
DOVIVdr *vdr;
int ret;
- uint8_t nal_prefix;
uint8_t rpu_type;
uint8_t vdr_seq_info_present;
uint8_t vdr_dm_metadata_present;
uint8_t use_prev_vdr_rpu;
uint8_t use_nlq;
uint8_t profile;
- if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
- return ret;
- /* Container header */
+ if (rpu_size < 5)
+ goto fail;
+
+ /* Container */
if (s->dv_profile == 10 /* dav1.10 */) {
/* DV inside AV1 re-uses an EMDF container skeleton, but with fixed
* values - so we can effectively treat this as a magic byte sequence.
@@ -229,18 +231,46 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
* discard_unknown_payload : f(1) = 1
*/
const unsigned header_magic = 0x01be6841u;
- unsigned header, emdf_payload_size;
- header = get_bits_long(gb, 27);
- VALIDATE(header, header_magic, header_magic);
+ unsigned emdf_header, emdf_payload_size, emdf_protection;
+ if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
+ return ret;
+ emdf_header = get_bits_long(gb, 27);
+ VALIDATE(emdf_header, header_magic, header_magic);
emdf_payload_size = get_variable_bits(gb, 8);
VALIDATE(emdf_payload_size, 6, 512);
if (emdf_payload_size * 8 > get_bits_left(gb))
return AVERROR_INVALIDDATA;
+
+ /* The payload is not byte-aligned (off by *one* bit, curse Dolby),
+ * so copy into a fresh buffer to preserve byte alignment of the
+ * RPU struct */
+ av_fast_padded_malloc(&s->rpu_buf, &s->rpu_buf_sz, emdf_payload_size);
+ if (!s->rpu_buf)
+ return AVERROR(ENOMEM);
+ for (int i = 0; i < emdf_payload_size; i++)
+ s->rpu_buf[i] = get_bits(gb, 8);
+ rpu = s->rpu_buf;
+ rpu_size = emdf_payload_size;
+
+ /* Validate EMDF footer */
+ emdf_protection = get_bits(gb, 5 + 12);
+ VALIDATE(emdf_protection, 0x400, 0x400);
+
+ if ((ret = init_get_bits8(gb, s->rpu_buf, emdf_payload_size)) < 0)
+ return ret;
} else {
- nal_prefix = get_bits(gb, 8);
- VALIDATE(nal_prefix, 25, 25);
+ /* NAL RBSP with prefix and trailing zeroes */
+ VALIDATE(rpu[0], 25, 25); /* NAL prefix */
+ rpu++;
+ rpu_size--;
+ /* Strip trailing padding bytes */
+ while (rpu_size && rpu[rpu_size - 1] == 0)
+ rpu_size--;
}
+ if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
+ return ret;
+
/* RPU header */
rpu_type = get_bits(gb, 6);
if (rpu_type != 2) {
diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
index 51c5fdbb879..506974a74bf 100644
--- a/libavcodec/dovi_rpu.h
+++ b/libavcodec/dovi_rpu.h
@@ -49,6 +49,8 @@ typedef struct DOVIContext {
*/
struct DOVIVdr *vdr[DOVI_MAX_DM_ID+1]; ///< RefStruct references
uint8_t dv_profile;
+ uint8_t *rpu_buf; ///< temporary buffer
+ unsigned rpu_buf_sz;
} DOVIContext;
--
2.44.0
_______________________________________________
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] 13+ messages in thread* Re: [FFmpeg-devel] [PATCH 3/6] avcodec/dovi_rpu: strip container in separate step
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 3/6] avcodec/dovi_rpu: strip container in separate step Niklas Haas
@ 2024-03-23 18:02 ` Andreas Rheinhardt
2024-03-23 18:06 ` Niklas Haas
0 siblings, 1 reply; 13+ messages in thread
From: Andreas Rheinhardt @ 2024-03-23 18:02 UTC (permalink / raw)
To: ffmpeg-devel
Niklas Haas:
> From: Niklas Haas <git@haasn.dev>
>
> This ensures that `gb` in the following section is fully byte-aligned,
> points at the start of the actual RPU, and ends on the CRC terminator.
>
> This is important for both calculation of the CRC, as well as dovi
> extension block parsing (which aligns to byte boundaries in various
> places).
> ---
> libavcodec/dovi_rpu.c | 48 +++++++++++++++++++++++++++++++++++--------
> libavcodec/dovi_rpu.h | 2 ++
> 2 files changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
> index c84a942f476..24a1cdf39a8 100644
> --- a/libavcodec/dovi_rpu.c
> +++ b/libavcodec/dovi_rpu.c
> @@ -23,6 +23,7 @@
>
> #include "libavutil/buffer.h"
>
> +#include "avcodec.h"
What is that used for?
> #include "dovi_rpu.h"
> #include "golomb.h"
> #include "get_bits.h"
> @@ -45,6 +46,7 @@ void ff_dovi_ctx_unref(DOVIContext *s)
> {
> for (int i = 0; i < FF_ARRAY_ELEMS(s->vdr); i++)
> ff_refstruct_unref(&s->vdr[i]);
> + av_free(s->rpu_buf);
>
> *s = (DOVIContext) {
> .logctx = s->logctx,
> @@ -202,17 +204,17 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
> DOVIVdr *vdr;
> int ret;
>
> - uint8_t nal_prefix;
> uint8_t rpu_type;
> uint8_t vdr_seq_info_present;
> uint8_t vdr_dm_metadata_present;
> uint8_t use_prev_vdr_rpu;
> uint8_t use_nlq;
> uint8_t profile;
> - if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
> - return ret;
>
> - /* Container header */
> + if (rpu_size < 5)
> + goto fail;
> +
> + /* Container */
> if (s->dv_profile == 10 /* dav1.10 */) {
> /* DV inside AV1 re-uses an EMDF container skeleton, but with fixed
> * values - so we can effectively treat this as a magic byte sequence.
> @@ -229,18 +231,46 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
> * discard_unknown_payload : f(1) = 1
> */
> const unsigned header_magic = 0x01be6841u;
> - unsigned header, emdf_payload_size;
> - header = get_bits_long(gb, 27);
> - VALIDATE(header, header_magic, header_magic);
> + unsigned emdf_header, emdf_payload_size, emdf_protection;
> + if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
> + return ret;
> + emdf_header = get_bits_long(gb, 27);
> + VALIDATE(emdf_header, header_magic, header_magic);
> emdf_payload_size = get_variable_bits(gb, 8);
> VALIDATE(emdf_payload_size, 6, 512);
> if (emdf_payload_size * 8 > get_bits_left(gb))
> return AVERROR_INVALIDDATA;
> +
> + /* The payload is not byte-aligned (off by *one* bit, curse Dolby),
> + * so copy into a fresh buffer to preserve byte alignment of the
> + * RPU struct */
> + av_fast_padded_malloc(&s->rpu_buf, &s->rpu_buf_sz, emdf_payload_size);
> + if (!s->rpu_buf)
> + return AVERROR(ENOMEM);
> + for (int i = 0; i < emdf_payload_size; i++)
> + s->rpu_buf[i] = get_bits(gb, 8);
> + rpu = s->rpu_buf;
> + rpu_size = emdf_payload_size;
> +
> + /* Validate EMDF footer */
> + emdf_protection = get_bits(gb, 5 + 12);
> + VALIDATE(emdf_protection, 0x400, 0x400);
> +
> + if ((ret = init_get_bits8(gb, s->rpu_buf, emdf_payload_size)) < 0)
> + return ret;
> } else {
> - nal_prefix = get_bits(gb, 8);
> - VALIDATE(nal_prefix, 25, 25);
> + /* NAL RBSP with prefix and trailing zeroes */
> + VALIDATE(rpu[0], 25, 25); /* NAL prefix */
> + rpu++;
> + rpu_size--;
> + /* Strip trailing padding bytes */
> + while (rpu_size && rpu[rpu_size - 1] == 0)
> + rpu_size--;
> }
>
> + if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
> + return ret;
> +
> /* RPU header */
> rpu_type = get_bits(gb, 6);
> if (rpu_type != 2) {
> diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
> index 51c5fdbb879..506974a74bf 100644
> --- a/libavcodec/dovi_rpu.h
> +++ b/libavcodec/dovi_rpu.h
> @@ -49,6 +49,8 @@ typedef struct DOVIContext {
> */
> struct DOVIVdr *vdr[DOVI_MAX_DM_ID+1]; ///< RefStruct references
> uint8_t dv_profile;
> + uint8_t *rpu_buf; ///< temporary buffer
> + unsigned rpu_buf_sz;
The order here introduces unnecessary padding.
>
> } DOVIContext;
>
_______________________________________________
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] 13+ messages in thread* Re: [FFmpeg-devel] [PATCH 3/6] avcodec/dovi_rpu: strip container in separate step
2024-03-23 18:02 ` Andreas Rheinhardt
@ 2024-03-23 18:06 ` Niklas Haas
0 siblings, 0 replies; 13+ messages in thread
From: Niklas Haas @ 2024-03-23 18:06 UTC (permalink / raw)
To: ffmpeg-devel
On Sat, 23 Mar 2024 19:02:26 +0100 Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
> Niklas Haas:
> > From: Niklas Haas <git@haasn.dev>
> >
> > This ensures that `gb` in the following section is fully byte-aligned,
> > points at the start of the actual RPU, and ends on the CRC terminator.
> >
> > This is important for both calculation of the CRC, as well as dovi
> > extension block parsing (which aligns to byte boundaries in various
> > places).
> > ---
> > libavcodec/dovi_rpu.c | 48 +++++++++++++++++++++++++++++++++++--------
> > libavcodec/dovi_rpu.h | 2 ++
> > 2 files changed, 41 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
> > index c84a942f476..24a1cdf39a8 100644
> > --- a/libavcodec/dovi_rpu.c
> > +++ b/libavcodec/dovi_rpu.c
> > @@ -23,6 +23,7 @@
> >
> > #include "libavutil/buffer.h"
> >
> > +#include "avcodec.h"
>
> What is that used for?
av_fast_padded_malloc
>
> > #include "dovi_rpu.h"
> > #include "golomb.h"
> > #include "get_bits.h"
> > @@ -45,6 +46,7 @@ void ff_dovi_ctx_unref(DOVIContext *s)
> > {
> > for (int i = 0; i < FF_ARRAY_ELEMS(s->vdr); i++)
> > ff_refstruct_unref(&s->vdr[i]);
> > + av_free(s->rpu_buf);
> >
> > *s = (DOVIContext) {
> > .logctx = s->logctx,
> > @@ -202,17 +204,17 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
> > DOVIVdr *vdr;
> > int ret;
> >
> > - uint8_t nal_prefix;
> > uint8_t rpu_type;
> > uint8_t vdr_seq_info_present;
> > uint8_t vdr_dm_metadata_present;
> > uint8_t use_prev_vdr_rpu;
> > uint8_t use_nlq;
> > uint8_t profile;
> > - if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
> > - return ret;
> >
> > - /* Container header */
> > + if (rpu_size < 5)
> > + goto fail;
> > +
> > + /* Container */
> > if (s->dv_profile == 10 /* dav1.10 */) {
> > /* DV inside AV1 re-uses an EMDF container skeleton, but with fixed
> > * values - so we can effectively treat this as a magic byte sequence.
> > @@ -229,18 +231,46 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
> > * discard_unknown_payload : f(1) = 1
> > */
> > const unsigned header_magic = 0x01be6841u;
> > - unsigned header, emdf_payload_size;
> > - header = get_bits_long(gb, 27);
> > - VALIDATE(header, header_magic, header_magic);
> > + unsigned emdf_header, emdf_payload_size, emdf_protection;
> > + if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
> > + return ret;
> > + emdf_header = get_bits_long(gb, 27);
> > + VALIDATE(emdf_header, header_magic, header_magic);
> > emdf_payload_size = get_variable_bits(gb, 8);
> > VALIDATE(emdf_payload_size, 6, 512);
> > if (emdf_payload_size * 8 > get_bits_left(gb))
> > return AVERROR_INVALIDDATA;
> > +
> > + /* The payload is not byte-aligned (off by *one* bit, curse Dolby),
> > + * so copy into a fresh buffer to preserve byte alignment of the
> > + * RPU struct */
> > + av_fast_padded_malloc(&s->rpu_buf, &s->rpu_buf_sz, emdf_payload_size);
> > + if (!s->rpu_buf)
> > + return AVERROR(ENOMEM);
> > + for (int i = 0; i < emdf_payload_size; i++)
> > + s->rpu_buf[i] = get_bits(gb, 8);
> > + rpu = s->rpu_buf;
> > + rpu_size = emdf_payload_size;
> > +
> > + /* Validate EMDF footer */
> > + emdf_protection = get_bits(gb, 5 + 12);
> > + VALIDATE(emdf_protection, 0x400, 0x400);
> > +
> > + if ((ret = init_get_bits8(gb, s->rpu_buf, emdf_payload_size)) < 0)
> > + return ret;
> > } else {
> > - nal_prefix = get_bits(gb, 8);
> > - VALIDATE(nal_prefix, 25, 25);
> > + /* NAL RBSP with prefix and trailing zeroes */
> > + VALIDATE(rpu[0], 25, 25); /* NAL prefix */
> > + rpu++;
> > + rpu_size--;
> > + /* Strip trailing padding bytes */
> > + while (rpu_size && rpu[rpu_size - 1] == 0)
> > + rpu_size--;
> > }
> >
> > + if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
> > + return ret;
> > +
> > /* RPU header */
> > rpu_type = get_bits(gb, 6);
> > if (rpu_type != 2) {
> > diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
> > index 51c5fdbb879..506974a74bf 100644
> > --- a/libavcodec/dovi_rpu.h
> > +++ b/libavcodec/dovi_rpu.h
> > @@ -49,6 +49,8 @@ typedef struct DOVIContext {
> > */
> > struct DOVIVdr *vdr[DOVI_MAX_DM_ID+1]; ///< RefStruct references
> > uint8_t dv_profile;
> > + uint8_t *rpu_buf; ///< temporary buffer
> > + unsigned rpu_buf_sz;
>
> The order here introduces unnecessary padding.
Fixed.
>
> >
> > } DOVIContext;
> >
>
> _______________________________________________
> 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".
_______________________________________________
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] 13+ messages in thread
* [FFmpeg-devel] [PATCH 4/6] avcodec/dovi_rpu: verify RPU data CRC32
2024-03-23 17:37 [FFmpeg-devel] [PATCH 1/6] avutil/dovi_meta: add AVDOVIDataMapping.nlq_pivots Niklas Haas
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 2/6] avutil/dovi_meta: add dolby vision extension blocks Niklas Haas
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 3/6] avcodec/dovi_rpu: strip container in separate step Niklas Haas
@ 2024-03-23 17:37 ` Niklas Haas
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 5/6] avcodec/dovi_rpu: add ext_blocks array to DOVIContext Niklas Haas
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 6/6] avcodec/dovi_rpu: parse extension blocks Niklas Haas
4 siblings, 0 replies; 13+ messages in thread
From: Niklas Haas @ 2024-03-23 17:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: quietvoid, Niklas Haas
From: Niklas Haas <git@haasn.dev>
The Dolby Vision RPU contains a CRC32 to validate the payload against.
The implementation is CRC32/MPEG-2.
The CRC is only verified with the AV_EF_CRCCHECK flag.
Co-authored-by: quietvoid <tcChlisop0@gmail.com>
---
libavcodec/av1dec.c | 3 ++-
libavcodec/dovi_rpu.c | 18 ++++++++++++++++--
libavcodec/dovi_rpu.h | 3 ++-
libavcodec/hevcdec.c | 3 ++-
libavcodec/libdav1d.c | 3 ++-
5 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 4d074c39087..bff921a12b2 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -1001,7 +1001,8 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame,
provider_oriented_code != 0x800)
break;
- ret = ff_dovi_rpu_parse(&s->dovi, gb.buffer, gb.buffer_end - gb.buffer);
+ ret = ff_dovi_rpu_parse(&s->dovi, gb.buffer, gb.buffer_end - gb.buffer,
+ avctx->err_recognition);
if (ret < 0) {
av_log(avctx, AV_LOG_WARNING, "Error parsing DOVI OBU.\n");
break; // ignore
diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index 24a1cdf39a8..cd54c8716dc 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -22,6 +22,7 @@
*/
#include "libavutil/buffer.h"
+#include "libavutil/crc.h"
#include "avcodec.h"
#include "dovi_rpu.h"
@@ -197,7 +198,8 @@ static inline unsigned get_variable_bits(GetBitContext *gb, int n)
} \
} while (0)
-int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
+int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
+ int err_recognition)
{
AVDOVIRpuDataHeader *hdr = &s->header;
GetBitContext *gb = &(GetBitContext){0};
@@ -268,6 +270,19 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
rpu_size--;
}
+ if (!rpu_size || rpu[rpu_size - 1] != 0x80)
+ goto fail;
+
+ if (err_recognition & AV_EF_CRCCHECK) {
+ uint32_t crc = av_bswap32(av_crc(av_crc_get_table(AV_CRC_32_IEEE),
+ -1, rpu, rpu_size - 1)); /* exclude 0x80 */
+ if (crc) {
+ av_log(s->logctx, AV_LOG_ERROR, "RPU CRC mismatch: %X\n", crc);
+ if (err_recognition & AV_EF_EXPLODE)
+ goto fail;
+ }
+ }
+
if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
return ret;
@@ -508,7 +523,6 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
color->source_diagonal = get_bits(gb, 10);
}
- /* FIXME: verify CRC32, requires implementation of AV_CRC_32_MPEG_2 */
return 0;
fail:
diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
index 506974a74bf..f88fbf9b558 100644
--- a/libavcodec/dovi_rpu.h
+++ b/libavcodec/dovi_rpu.h
@@ -79,7 +79,8 @@ void ff_dovi_update_cfg(DOVIContext *s, const AVDOVIDecoderConfigurationRecord *
*
* Returns 0 or an error code.
*/
-int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size);
+int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
+ int err_recognition);
/**
* Attach the decoded AVDOVIMetadata as side data to an AVFrame.
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 76aa6b45882..ebd70eda234 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3193,7 +3193,8 @@ static int decode_nal_units(HEVCContext *s, const uint8_t *buf, int length)
return AVERROR(ENOMEM);
memcpy(s->rpu_buf->data, nal->raw_data + 2, nal->raw_size - 2);
- ret = ff_dovi_rpu_parse(&s->dovi_ctx, nal->data + 2, nal->size - 2);
+ ret = ff_dovi_rpu_parse(&s->dovi_ctx, nal->data + 2, nal->size - 2,
+ s->avctx->err_recognition);
if (ret < 0) {
av_buffer_unref(&s->rpu_buf);
av_log(s->avctx, AV_LOG_WARNING, "Error parsing DOVI NAL unit.\n");
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 17b0743cf0c..af95e46c343 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -567,7 +567,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
provider_oriented_code != 0x800)
break;
- res = ff_dovi_rpu_parse(&dav1d->dovi, gb.buffer, gb.buffer_end - gb.buffer);
+ res = ff_dovi_rpu_parse(&dav1d->dovi, gb.buffer, gb.buffer_end - gb.buffer,
+ c->err_recognition);
if (res < 0) {
av_log(c, AV_LOG_WARNING, "Error parsing DOVI OBU.\n");
break; // ignore
--
2.44.0
_______________________________________________
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] 13+ messages in thread* [FFmpeg-devel] [PATCH 5/6] avcodec/dovi_rpu: add ext_blocks array to DOVIContext
2024-03-23 17:37 [FFmpeg-devel] [PATCH 1/6] avutil/dovi_meta: add AVDOVIDataMapping.nlq_pivots Niklas Haas
` (2 preceding siblings ...)
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 4/6] avcodec/dovi_rpu: verify RPU data CRC32 Niklas Haas
@ 2024-03-23 17:37 ` Niklas Haas
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 6/6] avcodec/dovi_rpu: parse extension blocks Niklas Haas
4 siblings, 0 replies; 13+ messages in thread
From: Niklas Haas @ 2024-03-23 17:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
---
libavcodec/dovi_rpu.c | 1 +
libavcodec/dovi_rpu.h | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index cd54c8716dc..21cb1850e3e 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -48,6 +48,7 @@ void ff_dovi_ctx_unref(DOVIContext *s)
for (int i = 0; i < FF_ARRAY_ELEMS(s->vdr); i++)
ff_refstruct_unref(&s->vdr[i]);
av_free(s->rpu_buf);
+ av_free(s->ext_blocks);
*s = (DOVIContext) {
.logctx = s->logctx,
diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
index f88fbf9b558..c632a83f46b 100644
--- a/libavcodec/dovi_rpu.h
+++ b/libavcodec/dovi_rpu.h
@@ -44,6 +44,12 @@ typedef struct DOVIContext {
const AVDOVIDataMapping *mapping;
const AVDOVIColorMetadata *color;
+ /**
+ * Currently active extension blocks, updates on every ff_dovi_rpu_parse()
+ */
+ AVDOVIDmData *ext_blocks;
+ int num_ext_blocks;
+
/**
* Private fields internal to dovi_rpu.c
*/
@@ -51,6 +57,7 @@ typedef struct DOVIContext {
uint8_t dv_profile;
uint8_t *rpu_buf; ///< temporary buffer
unsigned rpu_buf_sz;
+ unsigned ext_blocks_sz;
} DOVIContext;
--
2.44.0
_______________________________________________
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] 13+ messages in thread* [FFmpeg-devel] [PATCH 6/6] avcodec/dovi_rpu: parse extension blocks
2024-03-23 17:37 [FFmpeg-devel] [PATCH 1/6] avutil/dovi_meta: add AVDOVIDataMapping.nlq_pivots Niklas Haas
` (3 preceding siblings ...)
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 5/6] avcodec/dovi_rpu: add ext_blocks array to DOVIContext Niklas Haas
@ 2024-03-23 17:37 ` Niklas Haas
2024-03-23 18:00 ` Andreas Rheinhardt
4 siblings, 1 reply; 13+ messages in thread
From: Niklas Haas @ 2024-03-23 17:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: quietvoid, Niklas Haas
From: Niklas Haas <git@haasn.dev>
We split the inner loop between v1 and v2 extension blocks to print
a warning where an extension block was encountered in an unexpected
context.
Co-authored-by: quietvoid <tcChlisop0@gmail.com>
---
libavcodec/dovi_rpu.c | 178 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 178 insertions(+)
diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index 21cb1850e3e..99a983f65dd 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -199,6 +199,174 @@ static inline unsigned get_variable_bits(GetBitContext *gb, int n)
} \
} while (0)
+static void parse_ext_v1(DOVIContext *s, GetBitContext *gb, AVDOVIDmData *dm)
+{
+ switch (dm->level) {
+ case 1:
+ dm->l1.min_pq = get_bits(gb, 12);
+ dm->l1.max_pq = get_bits(gb, 12);
+ dm->l1.avg_pq = get_bits(gb, 12);
+ break;
+ case 2:
+ dm->l2.target_max_pq = get_bits(gb, 12);
+ dm->l2.trim_slope = get_bits(gb, 12);
+ dm->l2.trim_offset = get_bits(gb, 12);
+ dm->l2.trim_power = get_bits(gb, 12);
+ dm->l2.trim_chroma_weight = get_bits(gb, 12);
+ dm->l2.trim_saturation_gain = get_bits(gb, 12);
+ dm->l2.ms_weight = get_bits(gb, 13) - 8192;
+ break;
+ case 4:
+ dm->l4.anchor_pq = get_bits(gb, 12);
+ dm->l4.anchor_power = get_bits(gb, 12);
+ break;
+ case 5:
+ dm->l5.left_offset = get_bits(gb, 13);
+ dm->l5.right_offset = get_bits(gb, 13);
+ dm->l5.top_offset = get_bits(gb, 13);
+ dm->l5.bottom_offset = get_bits(gb, 13);
+ break;
+ case 6:
+ dm->l6.max_luminance = get_bits(gb, 16);
+ dm->l6.min_luminance = get_bits(gb, 16);
+ dm->l6.max_cll = get_bits(gb, 16);
+ dm->l6.max_fall = get_bits(gb, 16);
+ break;
+ case 255:
+ dm->l255.dm_run_mode = get_bits(gb, 8);
+ dm->l255.dm_run_version = get_bits(gb, 8);
+ for (int i = 0; i < 4; i++)
+ dm->l255.dm_debug[i] = get_bits(gb, 8);
+ break;
+ default:
+ av_log(s->logctx, AV_LOG_WARNING,
+ "Unknown Dolby Vision DM v1 level: %u\n", dm->level);
+ }
+}
+
+static inline AVCIExy get_cie_xy(GetBitContext *gb)
+{
+ const int denom = 32767;
+ return (AVCIExy) {
+ .x = { get_sbits(gb, 16), denom },
+ .y = { get_sbits(gb, 16), denom },
+ };
+}
+
+static void parse_ext_v2(DOVIContext *s, GetBitContext *gb, AVDOVIDmData *dm,
+ int ext_block_length)
+{
+ switch (dm->level) {
+ case 3:
+ dm->l3.min_pq_offset = get_bits(gb, 12);
+ dm->l3.max_pq_offset = get_bits(gb, 12);
+ dm->l3.avg_pq_offset = get_bits(gb, 12);
+ break;
+ case 8:
+ dm->l8.target_display_index = get_bits(gb, 8);
+ dm->l8.trim_slope = get_bits(gb, 12);
+ dm->l8.trim_offset = get_bits(gb, 12);
+ dm->l8.trim_power = get_bits(gb, 12);
+ dm->l8.trim_chroma_weight = get_bits(gb, 12);
+ dm->l8.trim_saturation_gain = get_bits(gb, 12);
+ dm->l8.ms_weight = get_bits(gb, 12) - 8192;
+ if (ext_block_length < 12)
+ break;
+ dm->l8.target_mid_contrast = get_bits(gb, 12);
+ if (ext_block_length < 13)
+ break;
+ dm->l8.clip_trim = get_bits(gb, 12);
+ if (ext_block_length < 19)
+ break;
+ for (int i = 0; i < 6; i++)
+ dm->l8.saturation_vector_field[i] = get_bits(gb, 8);
+ if (ext_block_length < 25)
+ break;
+ for (int i = 0; i < 6; i++)
+ dm->l8.hue_vector_field[i] = get_bits(gb, 8);
+ break;
+ case 9:
+ dm->l9.source_primary_index = get_bits(gb, 8);
+ if (ext_block_length < 17)
+ break;
+ dm->l9.source_display_primaries.prim.r = get_cie_xy(gb);
+ dm->l9.source_display_primaries.prim.g = get_cie_xy(gb);
+ dm->l9.source_display_primaries.prim.b = get_cie_xy(gb);
+ dm->l9.source_display_primaries.wp = get_cie_xy(gb);
+ break;
+ case 10:
+ dm->l10.target_display_index = get_bits(gb, 8);
+ dm->l10.target_max_pq = get_bits(gb, 12);
+ dm->l10.target_min_pq = get_bits(gb, 12);
+ dm->l10.target_primary_index = get_bits(gb, 8);
+ if (ext_block_length < 21)
+ break;
+ dm->l10.target_display_primaries.prim.r = get_cie_xy(gb);
+ dm->l10.target_display_primaries.prim.g = get_cie_xy(gb);
+ dm->l10.target_display_primaries.prim.b = get_cie_xy(gb);
+ dm->l10.target_display_primaries.wp = get_cie_xy(gb);
+ break;
+ case 11:
+ dm->l11.content_type = get_bits(gb, 8);
+ dm->l11.whitepoint = get_bits(gb, 4);
+ dm->l11.reference_mode_flag = get_bits(gb, 1);
+ skip_bits(gb, 3); /* reserved */
+ dm->l11.sharpness = get_bits(gb, 2);
+ dm->l11.noise_reduction = get_bits(gb, 2);
+ dm->l11.mpeg_noise_reduction = get_bits(gb, 2);
+ dm->l11.frame_rate_conversion = get_bits(gb, 2);
+ dm->l11.brightness = get_bits(gb, 2);
+ dm->l11.color = get_bits(gb, 2);
+ break;
+ case 254:
+ dm->l254.dm_mode = get_bits(gb, 8);
+ dm->l254.dm_version_index = get_bits(gb, 8);
+ break;
+ default:
+ av_log(s->logctx, AV_LOG_WARNING,
+ "Unknown Dolby Vision DM v2 level: %u\n", dm->level);
+ }
+}
+
+static int parse_ext_blocks(DOVIContext *s, GetBitContext *gb, int ver)
+{
+ int num_ext_blocks, ext_block_length, start_pos, parsed_bits;
+ AVDOVIDmData *ext_blocks;
+ size_t alloc_sz;
+
+ num_ext_blocks = get_ue_golomb_31(gb);
+ align_get_bits(gb);
+
+ alloc_sz = (s->num_ext_blocks + num_ext_blocks) * sizeof(AVDOVIDmData);
+ ext_blocks = av_fast_realloc(s->ext_blocks, &s->ext_blocks_sz, alloc_sz);
+ if (!ext_blocks) {
+ ff_dovi_ctx_unref(s);
+ return AVERROR(ENOMEM);
+ }
+ s->ext_blocks = ext_blocks;
+
+ while (num_ext_blocks--) {
+ AVDOVIDmData *dm = &s->ext_blocks[s->num_ext_blocks++];
+ ext_block_length = get_ue_golomb_31(gb);
+ dm->level = get_bits(gb, 8);
+ start_pos = get_bits_count(gb);
+
+ switch (ver) {
+ case 1: parse_ext_v1(s, gb, dm); break;
+ case 2: parse_ext_v2(s, gb, dm, ext_block_length); break;
+ }
+
+ parsed_bits = get_bits_count(gb) - start_pos;
+ if (parsed_bits > ext_block_length * 8) {
+ ff_dovi_ctx_unref(s);
+ return AVERROR_INVALIDDATA;
+ }
+ skip_bits(gb, ext_block_length * 8 - parsed_bits);
+ }
+
+ return 0;
+}
+
int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
int err_recognition)
{
@@ -524,6 +692,16 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
color->source_diagonal = get_bits(gb, 10);
}
+ /* Parse extension blocks */
+ s->num_ext_blocks = 0;
+ if ((ret = parse_ext_blocks(s, gb, 1)) < 0)
+ return ret;
+
+ if (get_bits_left(gb) > 48 /* padding + CRC32 + terminator */) {
+ if ((ret = parse_ext_blocks(s, gb, 2)) < 0)
+ return ret;
+ }
+
return 0;
fail:
--
2.44.0
_______________________________________________
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] 13+ messages in thread* Re: [FFmpeg-devel] [PATCH 6/6] avcodec/dovi_rpu: parse extension blocks
2024-03-23 17:37 ` [FFmpeg-devel] [PATCH 6/6] avcodec/dovi_rpu: parse extension blocks Niklas Haas
@ 2024-03-23 18:00 ` Andreas Rheinhardt
2024-03-23 18:23 ` Niklas Haas
0 siblings, 1 reply; 13+ messages in thread
From: Andreas Rheinhardt @ 2024-03-23 18:00 UTC (permalink / raw)
To: ffmpeg-devel
Niklas Haas:
> From: Niklas Haas <git@haasn.dev>
>
> We split the inner loop between v1 and v2 extension blocks to print
> a warning where an extension block was encountered in an unexpected
> context.
>
> Co-authored-by: quietvoid <tcChlisop0@gmail.com>
> ---
> libavcodec/dovi_rpu.c | 178 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 178 insertions(+)
>
> diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
> index 21cb1850e3e..99a983f65dd 100644
> --- a/libavcodec/dovi_rpu.c
> +++ b/libavcodec/dovi_rpu.c
> @@ -199,6 +199,174 @@ static inline unsigned get_variable_bits(GetBitContext *gb, int n)
> } \
> } while (0)
>
> +static void parse_ext_v1(DOVIContext *s, GetBitContext *gb, AVDOVIDmData *dm)
> +{
> + switch (dm->level) {
> + case 1:
> + dm->l1.min_pq = get_bits(gb, 12);
> + dm->l1.max_pq = get_bits(gb, 12);
> + dm->l1.avg_pq = get_bits(gb, 12);
> + break;
> + case 2:
> + dm->l2.target_max_pq = get_bits(gb, 12);
> + dm->l2.trim_slope = get_bits(gb, 12);
> + dm->l2.trim_offset = get_bits(gb, 12);
> + dm->l2.trim_power = get_bits(gb, 12);
> + dm->l2.trim_chroma_weight = get_bits(gb, 12);
> + dm->l2.trim_saturation_gain = get_bits(gb, 12);
> + dm->l2.ms_weight = get_bits(gb, 13) - 8192;
> + break;
> + case 4:
> + dm->l4.anchor_pq = get_bits(gb, 12);
> + dm->l4.anchor_power = get_bits(gb, 12);
> + break;
> + case 5:
> + dm->l5.left_offset = get_bits(gb, 13);
> + dm->l5.right_offset = get_bits(gb, 13);
> + dm->l5.top_offset = get_bits(gb, 13);
> + dm->l5.bottom_offset = get_bits(gb, 13);
> + break;
> + case 6:
> + dm->l6.max_luminance = get_bits(gb, 16);
> + dm->l6.min_luminance = get_bits(gb, 16);
> + dm->l6.max_cll = get_bits(gb, 16);
> + dm->l6.max_fall = get_bits(gb, 16);
> + break;
> + case 255:
> + dm->l255.dm_run_mode = get_bits(gb, 8);
> + dm->l255.dm_run_version = get_bits(gb, 8);
> + for (int i = 0; i < 4; i++)
> + dm->l255.dm_debug[i] = get_bits(gb, 8);
> + break;
> + default:
> + av_log(s->logctx, AV_LOG_WARNING,
> + "Unknown Dolby Vision DM v1 level: %u\n", dm->level);
> + }
> +}
> +
> +static inline AVCIExy get_cie_xy(GetBitContext *gb)
> +{
> + const int denom = 32767;
> + return (AVCIExy) {
> + .x = { get_sbits(gb, 16), denom },
> + .y = { get_sbits(gb, 16), denom },
> + };
The order of initializations is undefined.
> +}
> +
> +static void parse_ext_v2(DOVIContext *s, GetBitContext *gb, AVDOVIDmData *dm,
> + int ext_block_length)
> +{
> + switch (dm->level) {
> + case 3:
> + dm->l3.min_pq_offset = get_bits(gb, 12);
> + dm->l3.max_pq_offset = get_bits(gb, 12);
> + dm->l3.avg_pq_offset = get_bits(gb, 12);
> + break;
> + case 8:
> + dm->l8.target_display_index = get_bits(gb, 8);
> + dm->l8.trim_slope = get_bits(gb, 12);
> + dm->l8.trim_offset = get_bits(gb, 12);
> + dm->l8.trim_power = get_bits(gb, 12);
> + dm->l8.trim_chroma_weight = get_bits(gb, 12);
> + dm->l8.trim_saturation_gain = get_bits(gb, 12);
> + dm->l8.ms_weight = get_bits(gb, 12) - 8192;
> + if (ext_block_length < 12)
> + break;
> + dm->l8.target_mid_contrast = get_bits(gb, 12);
> + if (ext_block_length < 13)
> + break;
> + dm->l8.clip_trim = get_bits(gb, 12);
> + if (ext_block_length < 19)
> + break;
> + for (int i = 0; i < 6; i++)
> + dm->l8.saturation_vector_field[i] = get_bits(gb, 8);
> + if (ext_block_length < 25)
> + break;
> + for (int i = 0; i < 6; i++)
> + dm->l8.hue_vector_field[i] = get_bits(gb, 8);
> + break;
> + case 9:
> + dm->l9.source_primary_index = get_bits(gb, 8);
> + if (ext_block_length < 17)
> + break;
> + dm->l9.source_display_primaries.prim.r = get_cie_xy(gb);
> + dm->l9.source_display_primaries.prim.g = get_cie_xy(gb);
> + dm->l9.source_display_primaries.prim.b = get_cie_xy(gb);
> + dm->l9.source_display_primaries.wp = get_cie_xy(gb);
> + break;
> + case 10:
> + dm->l10.target_display_index = get_bits(gb, 8);
> + dm->l10.target_max_pq = get_bits(gb, 12);
> + dm->l10.target_min_pq = get_bits(gb, 12);
> + dm->l10.target_primary_index = get_bits(gb, 8);
> + if (ext_block_length < 21)
> + break;
> + dm->l10.target_display_primaries.prim.r = get_cie_xy(gb);
> + dm->l10.target_display_primaries.prim.g = get_cie_xy(gb);
> + dm->l10.target_display_primaries.prim.b = get_cie_xy(gb);
> + dm->l10.target_display_primaries.wp = get_cie_xy(gb);
> + break;
> + case 11:
> + dm->l11.content_type = get_bits(gb, 8);
> + dm->l11.whitepoint = get_bits(gb, 4);
> + dm->l11.reference_mode_flag = get_bits(gb, 1);
> + skip_bits(gb, 3); /* reserved */
> + dm->l11.sharpness = get_bits(gb, 2);
> + dm->l11.noise_reduction = get_bits(gb, 2);
> + dm->l11.mpeg_noise_reduction = get_bits(gb, 2);
> + dm->l11.frame_rate_conversion = get_bits(gb, 2);
> + dm->l11.brightness = get_bits(gb, 2);
> + dm->l11.color = get_bits(gb, 2);
> + break;
> + case 254:
> + dm->l254.dm_mode = get_bits(gb, 8);
> + dm->l254.dm_version_index = get_bits(gb, 8);
> + break;
> + default:
> + av_log(s->logctx, AV_LOG_WARNING,
> + "Unknown Dolby Vision DM v2 level: %u\n", dm->level);
> + }
> +}
> +
> +static int parse_ext_blocks(DOVIContext *s, GetBitContext *gb, int ver)
> +{
> + int num_ext_blocks, ext_block_length, start_pos, parsed_bits;
> + AVDOVIDmData *ext_blocks;
> + size_t alloc_sz;
> +
> + num_ext_blocks = get_ue_golomb_31(gb);
get_ue_golomb_31() currently returns small values when encountering
golomb values outside of the 0-31 range; but this is not guaranteed to
be so.
> + align_get_bits(gb);
> +
> + alloc_sz = (s->num_ext_blocks + num_ext_blocks) * sizeof(AVDOVIDmData);
> + ext_blocks = av_fast_realloc(s->ext_blocks, &s->ext_blocks_sz, alloc_sz);
> + if (!ext_blocks) {
> + ff_dovi_ctx_unref(s);
> + return AVERROR(ENOMEM);
> + }
> + s->ext_blocks = ext_blocks;
> +
> + while (num_ext_blocks--) {
> + AVDOVIDmData *dm = &s->ext_blocks[s->num_ext_blocks++];
> + ext_block_length = get_ue_golomb_31(gb);
> + dm->level = get_bits(gb, 8);
> + start_pos = get_bits_count(gb);
> +
> + switch (ver) {
> + case 1: parse_ext_v1(s, gb, dm); break;
> + case 2: parse_ext_v2(s, gb, dm, ext_block_length); break;
> + }
> +
> + parsed_bits = get_bits_count(gb) - start_pos;
> + if (parsed_bits > ext_block_length * 8) {
> + ff_dovi_ctx_unref(s);
> + return AVERROR_INVALIDDATA;
> + }
> + skip_bits(gb, ext_block_length * 8 - parsed_bits);
> + }
> +
> + return 0;
> +}
> +
> int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
> int err_recognition)
> {
> @@ -524,6 +692,16 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
> color->source_diagonal = get_bits(gb, 10);
> }
>
> + /* Parse extension blocks */
> + s->num_ext_blocks = 0;
> + if ((ret = parse_ext_blocks(s, gb, 1)) < 0)
> + return ret;
> +
> + if (get_bits_left(gb) > 48 /* padding + CRC32 + terminator */) {
> + if ((ret = parse_ext_blocks(s, gb, 2)) < 0)
> + return ret;
> + }
> +
> return 0;
>
> fail:
Why do you implement parsing for something that does not get used at all?
_______________________________________________
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] 13+ messages in thread* Re: [FFmpeg-devel] [PATCH 6/6] avcodec/dovi_rpu: parse extension blocks
2024-03-23 18:00 ` Andreas Rheinhardt
@ 2024-03-23 18:23 ` Niklas Haas
0 siblings, 0 replies; 13+ messages in thread
From: Niklas Haas @ 2024-03-23 18:23 UTC (permalink / raw)
To: ffmpeg-devel
On Sat, 23 Mar 2024 19:00:59 +0100 Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
> Niklas Haas:
> > From: Niklas Haas <git@haasn.dev>
> >
> > We split the inner loop between v1 and v2 extension blocks to print
> > a warning where an extension block was encountered in an unexpected
> > context.
> >
> > Co-authored-by: quietvoid <tcChlisop0@gmail.com>
> > ---
> > libavcodec/dovi_rpu.c | 178 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 178 insertions(+)
> >
> > diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
> > index 21cb1850e3e..99a983f65dd 100644
> > --- a/libavcodec/dovi_rpu.c
> > +++ b/libavcodec/dovi_rpu.c
> > @@ -199,6 +199,174 @@ static inline unsigned get_variable_bits(GetBitContext *gb, int n)
> > } \
> > } while (0)
> >
> > +static void parse_ext_v1(DOVIContext *s, GetBitContext *gb, AVDOVIDmData *dm)
> > +{
> > + switch (dm->level) {
> > + case 1:
> > + dm->l1.min_pq = get_bits(gb, 12);
> > + dm->l1.max_pq = get_bits(gb, 12);
> > + dm->l1.avg_pq = get_bits(gb, 12);
> > + break;
> > + case 2:
> > + dm->l2.target_max_pq = get_bits(gb, 12);
> > + dm->l2.trim_slope = get_bits(gb, 12);
> > + dm->l2.trim_offset = get_bits(gb, 12);
> > + dm->l2.trim_power = get_bits(gb, 12);
> > + dm->l2.trim_chroma_weight = get_bits(gb, 12);
> > + dm->l2.trim_saturation_gain = get_bits(gb, 12);
> > + dm->l2.ms_weight = get_bits(gb, 13) - 8192;
> > + break;
> > + case 4:
> > + dm->l4.anchor_pq = get_bits(gb, 12);
> > + dm->l4.anchor_power = get_bits(gb, 12);
> > + break;
> > + case 5:
> > + dm->l5.left_offset = get_bits(gb, 13);
> > + dm->l5.right_offset = get_bits(gb, 13);
> > + dm->l5.top_offset = get_bits(gb, 13);
> > + dm->l5.bottom_offset = get_bits(gb, 13);
> > + break;
> > + case 6:
> > + dm->l6.max_luminance = get_bits(gb, 16);
> > + dm->l6.min_luminance = get_bits(gb, 16);
> > + dm->l6.max_cll = get_bits(gb, 16);
> > + dm->l6.max_fall = get_bits(gb, 16);
> > + break;
> > + case 255:
> > + dm->l255.dm_run_mode = get_bits(gb, 8);
> > + dm->l255.dm_run_version = get_bits(gb, 8);
> > + for (int i = 0; i < 4; i++)
> > + dm->l255.dm_debug[i] = get_bits(gb, 8);
> > + break;
> > + default:
> > + av_log(s->logctx, AV_LOG_WARNING,
> > + "Unknown Dolby Vision DM v1 level: %u\n", dm->level);
> > + }
> > +}
> > +
> > +static inline AVCIExy get_cie_xy(GetBitContext *gb)
> > +{
> > + const int denom = 32767;
> > + return (AVCIExy) {
> > + .x = { get_sbits(gb, 16), denom },
> > + .y = { get_sbits(gb, 16), denom },
> > + };
>
> The order of initializations is undefined.
Fixed.
> > +}
> > +
> > +static void parse_ext_v2(DOVIContext *s, GetBitContext *gb, AVDOVIDmData *dm,
> > + int ext_block_length)
> > +{
> > + switch (dm->level) {
> > + case 3:
> > + dm->l3.min_pq_offset = get_bits(gb, 12);
> > + dm->l3.max_pq_offset = get_bits(gb, 12);
> > + dm->l3.avg_pq_offset = get_bits(gb, 12);
> > + break;
> > + case 8:
> > + dm->l8.target_display_index = get_bits(gb, 8);
> > + dm->l8.trim_slope = get_bits(gb, 12);
> > + dm->l8.trim_offset = get_bits(gb, 12);
> > + dm->l8.trim_power = get_bits(gb, 12);
> > + dm->l8.trim_chroma_weight = get_bits(gb, 12);
> > + dm->l8.trim_saturation_gain = get_bits(gb, 12);
> > + dm->l8.ms_weight = get_bits(gb, 12) - 8192;
> > + if (ext_block_length < 12)
> > + break;
> > + dm->l8.target_mid_contrast = get_bits(gb, 12);
> > + if (ext_block_length < 13)
> > + break;
> > + dm->l8.clip_trim = get_bits(gb, 12);
> > + if (ext_block_length < 19)
> > + break;
> > + for (int i = 0; i < 6; i++)
> > + dm->l8.saturation_vector_field[i] = get_bits(gb, 8);
> > + if (ext_block_length < 25)
> > + break;
> > + for (int i = 0; i < 6; i++)
> > + dm->l8.hue_vector_field[i] = get_bits(gb, 8);
> > + break;
> > + case 9:
> > + dm->l9.source_primary_index = get_bits(gb, 8);
> > + if (ext_block_length < 17)
> > + break;
> > + dm->l9.source_display_primaries.prim.r = get_cie_xy(gb);
> > + dm->l9.source_display_primaries.prim.g = get_cie_xy(gb);
> > + dm->l9.source_display_primaries.prim.b = get_cie_xy(gb);
> > + dm->l9.source_display_primaries.wp = get_cie_xy(gb);
> > + break;
> > + case 10:
> > + dm->l10.target_display_index = get_bits(gb, 8);
> > + dm->l10.target_max_pq = get_bits(gb, 12);
> > + dm->l10.target_min_pq = get_bits(gb, 12);
> > + dm->l10.target_primary_index = get_bits(gb, 8);
> > + if (ext_block_length < 21)
> > + break;
> > + dm->l10.target_display_primaries.prim.r = get_cie_xy(gb);
> > + dm->l10.target_display_primaries.prim.g = get_cie_xy(gb);
> > + dm->l10.target_display_primaries.prim.b = get_cie_xy(gb);
> > + dm->l10.target_display_primaries.wp = get_cie_xy(gb);
> > + break;
> > + case 11:
> > + dm->l11.content_type = get_bits(gb, 8);
> > + dm->l11.whitepoint = get_bits(gb, 4);
> > + dm->l11.reference_mode_flag = get_bits(gb, 1);
> > + skip_bits(gb, 3); /* reserved */
> > + dm->l11.sharpness = get_bits(gb, 2);
> > + dm->l11.noise_reduction = get_bits(gb, 2);
> > + dm->l11.mpeg_noise_reduction = get_bits(gb, 2);
> > + dm->l11.frame_rate_conversion = get_bits(gb, 2);
> > + dm->l11.brightness = get_bits(gb, 2);
> > + dm->l11.color = get_bits(gb, 2);
> > + break;
> > + case 254:
> > + dm->l254.dm_mode = get_bits(gb, 8);
> > + dm->l254.dm_version_index = get_bits(gb, 8);
> > + break;
> > + default:
> > + av_log(s->logctx, AV_LOG_WARNING,
> > + "Unknown Dolby Vision DM v2 level: %u\n", dm->level);
> > + }
> > +}
> > +
> > +static int parse_ext_blocks(DOVIContext *s, GetBitContext *gb, int ver)
> > +{
> > + int num_ext_blocks, ext_block_length, start_pos, parsed_bits;
> > + AVDOVIDmData *ext_blocks;
> > + size_t alloc_sz;
> > +
> > + num_ext_blocks = get_ue_golomb_31(gb);
>
> get_ue_golomb_31() currently returns small values when encountering
> golomb values outside of the 0-31 range; but this is not guaranteed to
> be so.
Fixed.
And in doing so, I noticed that the total number of blocks is actually
fairly constrained, so there's probably no point in allocating this
dynamically at all.
>
> > + align_get_bits(gb);
> > +
> > + alloc_sz = (s->num_ext_blocks + num_ext_blocks) * sizeof(AVDOVIDmData);
> > + ext_blocks = av_fast_realloc(s->ext_blocks, &s->ext_blocks_sz, alloc_sz);
> > + if (!ext_blocks) {
> > + ff_dovi_ctx_unref(s);
> > + return AVERROR(ENOMEM);
> > + }
> > + s->ext_blocks = ext_blocks;
> > +
> > + while (num_ext_blocks--) {
> > + AVDOVIDmData *dm = &s->ext_blocks[s->num_ext_blocks++];
> > + ext_block_length = get_ue_golomb_31(gb);
> > + dm->level = get_bits(gb, 8);
> > + start_pos = get_bits_count(gb);
> > +
> > + switch (ver) {
> > + case 1: parse_ext_v1(s, gb, dm); break;
> > + case 2: parse_ext_v2(s, gb, dm, ext_block_length); break;
> > + }
> > +
> > + parsed_bits = get_bits_count(gb) - start_pos;
> > + if (parsed_bits > ext_block_length * 8) {
> > + ff_dovi_ctx_unref(s);
> > + return AVERROR_INVALIDDATA;
> > + }
> > + skip_bits(gb, ext_block_length * 8 - parsed_bits);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
> > int err_recognition)
> > {
> > @@ -524,6 +692,16 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
> > color->source_diagonal = get_bits(gb, 10);
> > }
> >
> > + /* Parse extension blocks */
> > + s->num_ext_blocks = 0;
> > + if ((ret = parse_ext_blocks(s, gb, 1)) < 0)
> > + return ret;
> > +
> > + if (get_bits_left(gb) > 48 /* padding + CRC32 + terminator */) {
> > + if ((ret = parse_ext_blocks(s, gb, 2)) < 0)
> > + return ret;
> > + }
> > +
> > return 0;
> >
> > fail:
>
> Why do you implement parsing for something that does not get used at all?
Oops, I forgot to submit the commit that actually exposes this in
ff_dovi_attach_side_data()!
For context, there are at least two valid use-cases:
1. Downstream clients want access to these (e.g. VLC, mpv); libplacebo
currently uses custom code to parse the RPU but having it done inside
FFmpeg would be better for everybody involved.
2. For transcoding, we want to be able to preserve the information
inside the extension blocks. This requires parsing them in the first
place, so we can synthesize them back into binary form.
>
> _______________________________________________
> 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".
_______________________________________________
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] 13+ messages in thread