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 <jamr...@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);
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
