[FFmpeg-devel] [PATCH 04/11] avutil/frame: av_frame_side_data_new_struct()
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Thu Mar 6 16:50:27 EET 2025
James Almer:
> As well as the AV_SIDE_DATA_PROP_STRUCT prop, to define types that describe
> a fixed-size C struct and not a flat byte array.
> This excludes types like VIDEO_ENC_PARAMS as the struct it describes may have
> extra bytes allocated past the end of the struct.
>
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> libavutil/ambient_viewing_environment.c | 9 ++-
> libavutil/frame.h | 17 ++++++
> libavutil/mastering_display_metadata.c | 9 ++-
> libavutil/side_data.c | 77 +++++++++++++++++++++----
> libavutil/side_data.h | 4 ++
> libavutil/spherical.c | 10 +++-
> 6 files changed, 107 insertions(+), 19 deletions(-)
>
> diff --git a/libavutil/ambient_viewing_environment.c b/libavutil/ambient_viewing_environment.c
> index e359727776..ee2e9427cd 100644
> --- a/libavutil/ambient_viewing_environment.c
> +++ b/libavutil/ambient_viewing_environment.c
> @@ -20,9 +20,12 @@
>
> #include "ambient_viewing_environment.h"
> #include "mem.h"
> +#include "side_data.h"
>
> -static void get_defaults(AVAmbientViewingEnvironment *env)
> +void ff_ave_get_defaults(void *obj)
> {
> + AVAmbientViewingEnvironment *env = obj;
> +
> env->ambient_illuminance =
> env->ambient_light_x =
> env->ambient_light_y = (AVRational) { 0, 1 };
> @@ -35,7 +38,7 @@ AVAmbientViewingEnvironment *av_ambient_viewing_environment_alloc(size_t *size)
> if (!env)
> return NULL;
>
> - get_defaults(env);
> + ff_ave_get_defaults(env);
>
> if (size)
> *size = sizeof(*env);
> @@ -53,7 +56,7 @@ AVAmbientViewingEnvironment *av_ambient_viewing_environment_create_side_data(AVF
> return NULL;
>
> memset(side_data->data, 0, side_data->size);
> - get_defaults((AVAmbientViewingEnvironment *)side_data->data);
> + ff_ave_get_defaults(side_data->data);
>
> return (AVAmbientViewingEnvironment *)side_data->data;
> }
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index a707b35087..cd6b5194d1 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -304,6 +304,13 @@ enum AVSideDataProps {
> * or adjusted to the new layout.
> */
> AV_SIDE_DATA_PROP_CHANNEL_DEPENDENT = (1 << 4),
> +
> + /**
> + * Side data stores a fixed-size C struct and not a flat byte array.
> + * Entries of this type should be allocated with
> + * @ref av_frame_side_data_new_struct().
> + */
> + AV_SIDE_DATA_PROP_STRUCT = (1 << 5),
> };
>
> /**
> @@ -1121,6 +1128,16 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
> enum AVFrameSideDataType type,
> size_t size, unsigned int flags);
>
> +/**
> + * Variant of @ref av_frame_side_data_new() to add side data entires of a type
s/entires/entries/
> + * with the AV_SIDE_DATA_PROP_STRUCT prop.
Using AV_SIDE_DATA_PROP_STRUCT has a problem: It excludes trivial cases
like AV_FRAME_DATA_AFD, AV_FRAME_DATA_SKIP_SAMPLES and
AV_FRAME_DATA_VIEW_ID as well as not so trivial cases like
AV_FRAME_DATA_DOVI_METADATA. Maybe simply use a flag that says "can be
used with av_frame_side_data_new_struct()" instead.
> + *
> + * @return newly added side data on success, NULL on error.
> + */
> +AVFrameSideData *av_frame_side_data_new_struct(AVFrameSideData ***sd, int *nb_sd,
> + enum AVFrameSideDataType type,
> + unsigned int flags);
Can't you add a new parameter nb_elems to also handle all the side data
that only has a single array? This would then also cover
AV_FRAME_DATA_REGIONS_OF_INTEREST, AV_FRAME_DATA_VIDEO_ENC_PARAMS,
AV_FRAME_DATA_DETECTION_BBOXES, AV_FRAME_DATA_VIDEO_HINT.
AV_FRAME_DATA_A53_CC, AV_FRAME_DATA_ICC_PROFILE,
AV_FRAME_DATA_SEI_UNREGISTERED, AV_FRAME_DATA_DOVI_RPU_BUFFER and
AV_FRAME_DATA_LCEVC.
If you want to, I can write the patch for this.
> +
> /**
> * Add a new side data entry to an array from an existing AVBufferRef.
> *
> diff --git a/libavutil/mastering_display_metadata.c b/libavutil/mastering_display_metadata.c
> index dd37ed7d0e..4948f30523 100644
> --- a/libavutil/mastering_display_metadata.c
> +++ b/libavutil/mastering_display_metadata.c
> @@ -24,9 +24,12 @@
>
> #include "mastering_display_metadata.h"
> #include "mem.h"
> +#include "side_data.h"
>
> -static void get_defaults(AVMasteringDisplayMetadata *mastering)
> +void ff_mdm_get_defaults(void *obj)
> {
> + AVMasteringDisplayMetadata *mastering = obj;
> +
> for (int i = 0; i < 3; i++)
> for (int j = 0; j < 2; j++)
> mastering->display_primaries[i][j] = (AVRational) { 0, 1 };
> @@ -47,7 +50,7 @@ AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc_size(size_t *siz
> if (!mastering)
> return NULL;
>
> - get_defaults(mastering);
> + ff_mdm_get_defaults(mastering);
>
> if (size)
> *size = sizeof(*mastering);
> @@ -64,7 +67,7 @@ AVMasteringDisplayMetadata *av_mastering_display_metadata_create_side_data(AVFra
> return NULL;
>
> memset(side_data->data, 0, sizeof(AVMasteringDisplayMetadata));
> - get_defaults((AVMasteringDisplayMetadata *)side_data->data);
> + ff_mdm_get_defaults(side_data->data);
>
> return (AVMasteringDisplayMetadata *)side_data->data;
> }
> diff --git a/libavutil/side_data.c b/libavutil/side_data.c
> index beb8ea3212..49197e321d 100644
> --- a/libavutil/side_data.c
> +++ b/libavutil/side_data.c
> @@ -24,43 +24,78 @@
> #include "mem.h"
> #include "side_data.h"
>
> +// headers for struct sizes
> +#include "libavcodec/defs.h"
> +#include "ambient_viewing_environment.h"
> +#include "downmix_info.h"
> +#include "hdr_dynamic_metadata.h"
> +#include "hdr_dynamic_vivid_metadata.h"
> +#include "mastering_display_metadata.h"
> +#include "motion_vector.h"
> +#include "replaygain.h"
> +#include "spherical.h"
> +#include "stereo3d.h"
> +
> typedef struct FFSideDataDescriptor {
> AVSideDataDescriptor p;
> +
> + void (*init)(void *obj);
Can't you use a switch instead to avoid the relocations?
> +
> + size_t size;
> } FFSideDataDescriptor;
>
> static const FFSideDataDescriptor sd_props[] = {
> - [AV_FRAME_DATA_PANSCAN] = { .p = { "AVPanScan", AV_SIDE_DATA_PROP_SIZE_DEPENDENT } },
> + [AV_FRAME_DATA_PANSCAN] = { .p = { "AVPanScan", AV_SIDE_DATA_PROP_STRUCT | AV_SIDE_DATA_PROP_SIZE_DEPENDENT },
> + .size = sizeof(AVPanScan) },
> [AV_FRAME_DATA_A53_CC] = { .p = { "ATSC A53 Part 4 Closed Captions" } },
> [AV_FRAME_DATA_MATRIXENCODING] = { .p = { "AVMatrixEncoding", AV_SIDE_DATA_PROP_CHANNEL_DEPENDENT } },
> - [AV_FRAME_DATA_DOWNMIX_INFO] = { .p = { "Metadata relevant to a downmix procedure", AV_SIDE_DATA_PROP_CHANNEL_DEPENDENT } },
> + [AV_FRAME_DATA_DOWNMIX_INFO] = { .p = { "Metadata relevant to a downmix procedure", AV_SIDE_DATA_PROP_STRUCT | AV_SIDE_DATA_PROP_CHANNEL_DEPENDENT },
> + .size = sizeof(AVDownmixInfo) },
> [AV_FRAME_DATA_AFD] = { .p = { "Active format description" } },
> - [AV_FRAME_DATA_MOTION_VECTORS] = { .p = { "Motion vectors", AV_SIDE_DATA_PROP_SIZE_DEPENDENT } },
> + [AV_FRAME_DATA_MOTION_VECTORS] = { .p = { "Motion vectors", AV_SIDE_DATA_PROP_STRUCT | AV_SIDE_DATA_PROP_SIZE_DEPENDENT },
> + .size = sizeof(AVMotionVector) },
> [AV_FRAME_DATA_SKIP_SAMPLES] = { .p = { "Skip samples" } },
> [AV_FRAME_DATA_GOP_TIMECODE] = { .p = { "GOP timecode" } },
> [AV_FRAME_DATA_S12M_TIMECODE] = { .p = { "SMPTE 12-1 timecode" } },
> - [AV_FRAME_DATA_DYNAMIC_HDR_PLUS] = { .p = { "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)", AV_SIDE_DATA_PROP_COLOR_DEPENDENT } },
> - [AV_FRAME_DATA_DYNAMIC_HDR_VIVID] = { .p = { "HDR Dynamic Metadata CUVA 005.1 2021 (Vivid)", AV_SIDE_DATA_PROP_COLOR_DEPENDENT } },
> + [AV_FRAME_DATA_DYNAMIC_HDR_PLUS] = { .p = { "HDR Dynamic Metadata SMPTE2094-40 (HDR10+)", AV_SIDE_DATA_PROP_STRUCT| AV_SIDE_DATA_PROP_COLOR_DEPENDENT },
> + .size = sizeof(AVDynamicHDRPlus) },
> + [AV_FRAME_DATA_DYNAMIC_HDR_VIVID] = { .p = { "HDR Dynamic Metadata CUVA 005.1 2021 (Vivid)", AV_SIDE_DATA_PROP_STRUCT | AV_SIDE_DATA_PROP_COLOR_DEPENDENT },
> + .size = sizeof(AVDynamicHDRVivid) },
> [AV_FRAME_DATA_REGIONS_OF_INTEREST] = { .p = { "Regions Of Interest", AV_SIDE_DATA_PROP_SIZE_DEPENDENT } },
> [AV_FRAME_DATA_VIDEO_ENC_PARAMS] = { .p = { "Video encoding parameters" } },
> - [AV_FRAME_DATA_FILM_GRAIN_PARAMS] = { .p = { "Film grain parameters" } },
> + [AV_FRAME_DATA_FILM_GRAIN_PARAMS] = { .p = { "Film grain parameters", AV_SIDE_DATA_PROP_STRUCT } },
> [AV_FRAME_DATA_DETECTION_BBOXES] = { .p = { "Bounding boxes for object detection and classification", AV_SIDE_DATA_PROP_SIZE_DEPENDENT } },
> [AV_FRAME_DATA_DOVI_RPU_BUFFER] = { .p = { "Dolby Vision RPU Data", AV_SIDE_DATA_PROP_COLOR_DEPENDENT } },
> [AV_FRAME_DATA_DOVI_METADATA] = { .p = { "Dolby Vision Metadata", AV_SIDE_DATA_PROP_COLOR_DEPENDENT } },
> [AV_FRAME_DATA_LCEVC] = { .p = { "LCEVC NAL data", AV_SIDE_DATA_PROP_SIZE_DEPENDENT } },
> [AV_FRAME_DATA_VIEW_ID] = { .p = { "View ID" } },
> - [AV_FRAME_DATA_STEREO3D] = { .p = { "Stereo 3D", AV_SIDE_DATA_PROP_GLOBAL } },
> - [AV_FRAME_DATA_REPLAYGAIN] = { .p = { "AVReplayGain", AV_SIDE_DATA_PROP_GLOBAL } },
> + [AV_FRAME_DATA_STEREO3D] = { .p = { "Stereo 3D", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_STRUCT },
> + .size = sizeof(AVStereo3D) },
> + [AV_FRAME_DATA_REPLAYGAIN] = { .p = { "AVReplayGain", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_STRUCT },
> + .size = sizeof(AVReplayGain) },
> [AV_FRAME_DATA_DISPLAYMATRIX] = { .p = { "3x3 displaymatrix", AV_SIDE_DATA_PROP_GLOBAL } },
> [AV_FRAME_DATA_AUDIO_SERVICE_TYPE] = { .p = { "Audio service type", AV_SIDE_DATA_PROP_GLOBAL } },
> - [AV_FRAME_DATA_MASTERING_DISPLAY_METADATA] = { .p = { "Mastering display metadata", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_COLOR_DEPENDENT } },
> - [AV_FRAME_DATA_CONTENT_LIGHT_LEVEL] = { .p = { "Content light level metadata", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_COLOR_DEPENDENT } },
> - [AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT] = { .p = { "Ambient viewing environment", AV_SIDE_DATA_PROP_GLOBAL } },
> - [AV_FRAME_DATA_SPHERICAL] = { .p = { "Spherical Mapping", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_SIZE_DEPENDENT } },
> + [AV_FRAME_DATA_MASTERING_DISPLAY_METADATA] = { .p = { "Mastering display metadata", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_STRUCT | AV_SIDE_DATA_PROP_COLOR_DEPENDENT },
> + .init = ff_mdm_get_defaults,
> + .size = sizeof(AVMasteringDisplayMetadata) },
> + [AV_FRAME_DATA_CONTENT_LIGHT_LEVEL] = { .p = { "Content light level metadata", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_STRUCT | AV_SIDE_DATA_PROP_COLOR_DEPENDENT },
> + .size = sizeof(AVContentLightMetadata) },
> + [AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT] = { .p = { "Ambient viewing environment", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_STRUCT },
> + .init = ff_ave_get_defaults,
> + .size = sizeof(AVAmbientViewingEnvironment) },
> + [AV_FRAME_DATA_SPHERICAL] = { .p = { "Spherical Mapping", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_STRUCT | AV_SIDE_DATA_PROP_SIZE_DEPENDENT },
> + .init = ff_spherical_get_defaults,
> + .size = sizeof(AVSphericalMapping) },
> [AV_FRAME_DATA_ICC_PROFILE] = { .p = { "ICC profile", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_COLOR_DEPENDENT } },
> [AV_FRAME_DATA_SEI_UNREGISTERED] = { .p = { "H.26[45] User Data Unregistered SEI message", AV_SIDE_DATA_PROP_MULTI } },
> [AV_FRAME_DATA_VIDEO_HINT] = { .p = { "Encoding video hint", AV_SIDE_DATA_PROP_SIZE_DEPENDENT } },
> };
>
> +static const FFSideDataDescriptor *dp_from_desc(const AVSideDataDescriptor *desc)
> +{
> + return (const FFSideDataDescriptor *)desc;
> +}
> +
> const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType type)
> {
> unsigned t = type;
> @@ -247,6 +282,24 @@ AVFrameSideData *av_frame_side_data_add(AVFrameSideData ***sd, int *nb_sd,
> return sd_dst;
> }
>
> +AVFrameSideData *av_frame_side_data_new_struct(AVFrameSideData ***sd, int *nb_sd,
> + enum AVFrameSideDataType type,
> + unsigned int flags)
> +{
> + const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
> + const FFSideDataDescriptor *dp = dp_from_desc(desc);
> + AVFrameSideData *ret;
> +
> + if (!desc || !(desc->props & AV_SIDE_DATA_PROP_STRUCT))
> + return NULL;
> +
> + av_assert0(dp->size);
> + ret = av_frame_side_data_new(sd, nb_sd, type, dp->size, flags);
> + if (ret && dp->init)
> + dp->init(ret->data);
> + return ret;
> +}
> +
> int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
> const AVFrameSideData *src, unsigned int flags)
> {
> diff --git a/libavutil/side_data.h b/libavutil/side_data.h
> index 8275aa35a5..5d25833882 100644
> --- a/libavutil/side_data.h
> +++ b/libavutil/side_data.h
> @@ -27,4 +27,8 @@ AVFrameSideData *ff_frame_side_data_add_from_buf(AVFrameSideData ***sd,
> enum AVFrameSideDataType type,
> AVBufferRef *buf);
>
> +void ff_mdm_get_defaults(void *obj);
> +void ff_ave_get_defaults(void *obj);
> +void ff_spherical_get_defaults(void *obj);
> +
> #endif // AVUTIL_SIDE_DATA_H
> diff --git a/libavutil/spherical.c b/libavutil/spherical.c
> index 64ade1d0ec..4ef2c225c2 100644
> --- a/libavutil/spherical.c
> +++ b/libavutil/spherical.c
> @@ -21,15 +21,23 @@
> #include "avstring.h"
> #include "macros.h"
> #include "mem.h"
> +#include "side_data.h"
> #include "spherical.h"
>
> +void ff_spherical_get_defaults(void *obj)
> +{
> + AVSphericalMapping *spherical = obj;
> +
> + spherical->projection = AV_SPHERICAL_RECTILINEAR;
> +}
> +
Can't this be inlined?
> AVSphericalMapping *av_spherical_alloc(size_t *size)
> {
> AVSphericalMapping *spherical = av_mallocz(sizeof(AVSphericalMapping));
> if (!spherical)
> return NULL;
>
> - spherical->projection = AV_SPHERICAL_RECTILINEAR;
> + ff_spherical_get_defaults(spherical);
>
> if (size)
> *size = sizeof(*spherical);
More information about the ffmpeg-devel
mailing list