On Sun, Aug 20, 2023 at 12:44 PM Andreas Rheinhardt <andreas.rheinha...@outlook.com> wrote: > > Jan Ekström: > > --- > > libavutil/frame.c | 23 +++++++++++++++++++++++ > > libavutil/frame.h | 16 ++++++++++++++++ > > 2 files changed, 39 insertions(+) > > > > diff --git a/libavutil/frame.c b/libavutil/frame.c > > index d8910a2120..04d56853f0 100644 > > --- a/libavutil/frame.c > > +++ b/libavutil/frame.c > > @@ -880,6 +880,29 @@ AVFrameSideData > > *av_side_data_set_new_item(AVFrameSideDataSet *set, > > return ret; > > } > > > > +int av_side_data_set_extend(AVFrameSideDataSet *dst, > > + const AVFrameSideDataSet src, > > + unsigned int allow_duplicates) > > +{ > > + for (int i = 0; i < src.nb_sd; i++) { > > + const AVFrameSideData *sd_src = src.sd[i]; > > + AVFrameSideData *sd_dst = > > + av_side_data_set_new_item(dst, sd_src->type, > > + sd_src->size, > > + allow_duplicates); > > + if (!sd_dst) { > > + av_side_data_set_uninit(dst); > > + return AVERROR(ENOMEM); > > + } > > + > > + memcpy(sd_dst->data, sd_src->data, sd_src->size); > > AVFrame side data is reference-counted. >
Seems like I based this on av_frame_copy_props, so the force_copy=1 case. I guess following av_frame_ref makes more sense to follow. > > + > > + av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0); > > Missing check. > This comes straight out of frame_copy_props, so it seems like at least a couple of av_dict_copy calls within it are unchecked :) . I guess that warrants a separate fixup patch. > > + } > > + > > + return 0; > > +} > > + > > AVFrameSideData *av_side_data_set_get_item(const AVFrameSideDataSet set, > > enum AVFrameSideDataType type) > > { > > diff --git a/libavutil/frame.h b/libavutil/frame.h > > index 0cafc9c51f..2413000c9a 100644 > > --- a/libavutil/frame.h > > +++ b/libavutil/frame.h > > @@ -1083,6 +1083,22 @@ AVFrameSideData > > *av_side_data_set_new_item(AVFrameSideDataSet *set, > > size_t size, > > unsigned int allow_duplicates); > > > > +/** > > + * Add multiple side data entries to a set in one go. > > + * > > + * @param dst a set to which the side data should be added > > + * @param src a set from which the side data should be copied from > > This needs to add something to ensure that dst and src refer to > different side-data (i.e. to disallow calls like > AVFrameSideDataSet set = { ... }; > > av_side_data_set_extend(&set, set, 0);) > Sure, but interestingly enough only av_frame_replace currently has something like this. av_frame_ref does check that dst has zero width/height or channel count, but does not attempt to check that it is being called with different contents. > > + * @param allow_duplicates an unsigned integer noting whether duplicates > > are > > + * allowed or not. If duplicates are not allowed, > > all > > + * entries of the same side data type are first > > removed > > + * and freed before the new entry is added. > > Better use flags. > Done, the current state of the branch as I go through people's reviews is available at https://github.com/jeeb/ffmpeg/commits/avcodec_cll_mdcv_side_data_v4 . > > + * > > + * @return negative error code on failure, >=0 on success. > > + */ > > +int av_side_data_set_extend(AVFrameSideDataSet *dst, > > + const AVFrameSideDataSet src, > > + unsigned int allow_duplicates); > > Do we really need this function? Are there enough potential users of it? > I mostly added this for ffmpeg.c and if we want other API clients to do something similar, there should be a public function to allow for following similar behavior. Or would you rather prefer a function related to avctx that takes in an AVFrame, which does something like this internally? Best regards, Jan _______________________________________________ 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".