Message ID | 20230901203828.275197-9-jeebjp@gmail.com |
---|---|
State | New |
Headers | show |
Series | encoder AVCodecContext configuration side data | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
On 9/1/2023 5:38 PM, Jan Ekström wrote: > Additionally, extend the side data set FATE test to check for the > invalid use case of extending a set by itself. > --- > libavutil/frame.c | 32 ++++++++++++++++++++++++++++++++ > libavutil/frame.h | 15 +++++++++++++++ > libavutil/tests/side_data_set.c | 16 ++++++++++++++++ > tests/ref/fate/side_data_set | 7 +++++++ > 4 files changed, 70 insertions(+) > > diff --git a/libavutil/frame.c b/libavutil/frame.c > index 5f74e0172b..898c749631 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -879,6 +879,38 @@ AVFrameSideData *av_frame_side_data_set_new_item(AVFrameSideDataSet *set, > return ret; > } > > +int av_frame_side_data_set_extend(AVFrameSideDataSet *dst, > + const AVFrameSideDataSet src, > + unsigned int flags) flags is unused. Shouldn't you check for the no duplicates one? > +{ > + if (src.nb_sd > 0 && src.nb_sd == dst->nb_sd && > + src.sd == dst->sd) > + return AVERROR(EINVAL); > + > + for (int i = 0; i < src.nb_sd; i++) { > + const AVFrameSideData *sd_src = src.sd[i]; > + AVBufferRef *buf = av_buffer_ref(sd_src->buf); > + AVFrameSideData *sd_dst = > + add_side_data_to_set_from_buf(dst, sd_src->type, buf); > + if (!sd_dst) { > + av_buffer_unref(&buf); > + av_frame_side_data_set_uninit(dst); > + return AVERROR(ENOMEM); > + } > + > + { > + int ret = av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0); > + if (ret < 0) { > + av_frame_side_data_set_uninit(dst); > + return ret; > + } > + } > + > + } > + > + return 0; > +} > + > AVFrameSideData *av_frame_side_data_set_get_item(const AVFrameSideDataSet set, > enum AVFrameSideDataType type) > { > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 8ecdf82f33..e16941c5a5 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -1085,6 +1085,21 @@ AVFrameSideData *av_frame_side_data_set_new_item(AVFrameSideDataSet *set, > size_t size, > unsigned int flags); > > +/** > + * 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 > + * @param flags Some combination of AV_FRAME_SIDE_DATA_SET_FLAG_* flags, or 0. > + * > + * @return negative error code on failure, >=0 on success. > + * > + * @see av_frame_side_data_set_new_item regarding the flags. > + */ > +int av_frame_side_data_set_extend(AVFrameSideDataSet *dst, > + const AVFrameSideDataSet src, > + unsigned int flags); > + > /** > * Get a side data entry of a specific type from a set. > * > diff --git a/libavutil/tests/side_data_set.c b/libavutil/tests/side_data_set.c > index 056d79f655..0c9ceed962 100644 > --- a/libavutil/tests/side_data_set.c > +++ b/libavutil/tests/side_data_set.c > @@ -91,6 +91,22 @@ int main(void) > puts("\nFinal state after a single 'no-duplicates' addition:"); > print_clls(set); > > + { > + AVFrameSideDataSet dst_set = { 0 }; > + av_assert0(av_frame_side_data_set_extend(&dst_set, set, 0) >= 0); > + > + puts("\nState of the copied set:"); > + print_clls(dst_set); > + > + av_frame_side_data_set_uninit(&dst_set); > + } > + > + { > + int ret = av_frame_side_data_set_extend(&set, set, 0); > + printf("\nResult of trying to extend a set by itself: %s\n", > + av_err2str(ret)); > + } > + > av_frame_side_data_set_uninit(&set); > > return 0; > diff --git a/tests/ref/fate/side_data_set b/tests/ref/fate/side_data_set > index 7d8c684d8f..3050b31014 100644 > --- a/tests/ref/fate/side_data_set > +++ b/tests/ref/fate/side_data_set > @@ -12,3 +12,10 @@ Final state after a single 'no-duplicates' addition: > sd 0, Ambient viewing environment > sd 1, Spherical Mapping > sd 2, Content light level metadata: MaxCLL: 1337 > + > +State of the copied set: > +sd 0, Ambient viewing environment > +sd 1, Spherical Mapping > +sd 2, Content light level metadata: MaxCLL: 1337 > + > +Result of trying to extend a set by itself: Invalid argument
On 9/1/2023 5:38 PM, Jan Ekström wrote: > Additionally, extend the side data set FATE test to check for the > invalid use case of extending a set by itself. > --- > libavutil/frame.c | 32 ++++++++++++++++++++++++++++++++ > libavutil/frame.h | 15 +++++++++++++++ > libavutil/tests/side_data_set.c | 16 ++++++++++++++++ > tests/ref/fate/side_data_set | 7 +++++++ > 4 files changed, 70 insertions(+) > > diff --git a/libavutil/frame.c b/libavutil/frame.c > index 5f74e0172b..898c749631 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -879,6 +879,38 @@ AVFrameSideData *av_frame_side_data_set_new_item(AVFrameSideDataSet *set, > return ret; > } > > +int av_frame_side_data_set_extend(AVFrameSideDataSet *dst, > + const AVFrameSideDataSet src, > + unsigned int flags) I think a standard, more versatile add() function would be better. The signature would be something like av_frame_side_data_set_add_item(AVFrameSideDataSet *set, const AVBufferRef *buf, unsigned int flags); It would also be a good chance to fix the non standard behavior i had misfortune to introduce with av_frame_new_side_data_from_buf() by having this function actually create a new reference to the passed in buffer rather than take ownership of it. > +{ > + if (src.nb_sd > 0 && src.nb_sd == dst->nb_sd && > + src.sd == dst->sd) > + return AVERROR(EINVAL); > + > + for (int i = 0; i < src.nb_sd; i++) { > + const AVFrameSideData *sd_src = src.sd[i]; > + AVBufferRef *buf = av_buffer_ref(sd_src->buf); > + AVFrameSideData *sd_dst = > + add_side_data_to_set_from_buf(dst, sd_src->type, buf); > + if (!sd_dst) { > + av_buffer_unref(&buf); > + av_frame_side_data_set_uninit(dst); > + return AVERROR(ENOMEM); > + } > + > + { > + int ret = av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0); > + if (ret < 0) { > + av_frame_side_data_set_uninit(dst); > + return ret; > + } > + } > + > + } > + > + return 0; > +} > + > AVFrameSideData *av_frame_side_data_set_get_item(const AVFrameSideDataSet set, > enum AVFrameSideDataType type) > { > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 8ecdf82f33..e16941c5a5 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -1085,6 +1085,21 @@ AVFrameSideData *av_frame_side_data_set_new_item(AVFrameSideDataSet *set, > size_t size, > unsigned int flags); > > +/** > + * 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 > + * @param flags Some combination of AV_FRAME_SIDE_DATA_SET_FLAG_* flags, or 0. > + * > + * @return negative error code on failure, >=0 on success. > + * > + * @see av_frame_side_data_set_new_item regarding the flags. > + */ > +int av_frame_side_data_set_extend(AVFrameSideDataSet *dst, > + const AVFrameSideDataSet src, > + unsigned int flags); > + > /** > * Get a side data entry of a specific type from a set. > * > diff --git a/libavutil/tests/side_data_set.c b/libavutil/tests/side_data_set.c > index 056d79f655..0c9ceed962 100644 > --- a/libavutil/tests/side_data_set.c > +++ b/libavutil/tests/side_data_set.c > @@ -91,6 +91,22 @@ int main(void) > puts("\nFinal state after a single 'no-duplicates' addition:"); > print_clls(set); > > + { > + AVFrameSideDataSet dst_set = { 0 }; > + av_assert0(av_frame_side_data_set_extend(&dst_set, set, 0) >= 0); > + > + puts("\nState of the copied set:"); > + print_clls(dst_set); > + > + av_frame_side_data_set_uninit(&dst_set); > + } > + > + { > + int ret = av_frame_side_data_set_extend(&set, set, 0); > + printf("\nResult of trying to extend a set by itself: %s\n", > + av_err2str(ret)); > + } > + > av_frame_side_data_set_uninit(&set); > > return 0; > diff --git a/tests/ref/fate/side_data_set b/tests/ref/fate/side_data_set > index 7d8c684d8f..3050b31014 100644 > --- a/tests/ref/fate/side_data_set > +++ b/tests/ref/fate/side_data_set > @@ -12,3 +12,10 @@ Final state after a single 'no-duplicates' addition: > sd 0, Ambient viewing environment > sd 1, Spherical Mapping > sd 2, Content light level metadata: MaxCLL: 1337 > + > +State of the copied set: > +sd 0, Ambient viewing environment > +sd 1, Spherical Mapping > +sd 2, Content light level metadata: MaxCLL: 1337 > + > +Result of trying to extend a set by itself: Invalid argument
diff --git a/libavutil/frame.c b/libavutil/frame.c index 5f74e0172b..898c749631 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -879,6 +879,38 @@ AVFrameSideData *av_frame_side_data_set_new_item(AVFrameSideDataSet *set, return ret; } +int av_frame_side_data_set_extend(AVFrameSideDataSet *dst, + const AVFrameSideDataSet src, + unsigned int flags) +{ + if (src.nb_sd > 0 && src.nb_sd == dst->nb_sd && + src.sd == dst->sd) + return AVERROR(EINVAL); + + for (int i = 0; i < src.nb_sd; i++) { + const AVFrameSideData *sd_src = src.sd[i]; + AVBufferRef *buf = av_buffer_ref(sd_src->buf); + AVFrameSideData *sd_dst = + add_side_data_to_set_from_buf(dst, sd_src->type, buf); + if (!sd_dst) { + av_buffer_unref(&buf); + av_frame_side_data_set_uninit(dst); + return AVERROR(ENOMEM); + } + + { + int ret = av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0); + if (ret < 0) { + av_frame_side_data_set_uninit(dst); + return ret; + } + } + + } + + return 0; +} + AVFrameSideData *av_frame_side_data_set_get_item(const AVFrameSideDataSet set, enum AVFrameSideDataType type) { diff --git a/libavutil/frame.h b/libavutil/frame.h index 8ecdf82f33..e16941c5a5 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -1085,6 +1085,21 @@ AVFrameSideData *av_frame_side_data_set_new_item(AVFrameSideDataSet *set, size_t size, unsigned int flags); +/** + * 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 + * @param flags Some combination of AV_FRAME_SIDE_DATA_SET_FLAG_* flags, or 0. + * + * @return negative error code on failure, >=0 on success. + * + * @see av_frame_side_data_set_new_item regarding the flags. + */ +int av_frame_side_data_set_extend(AVFrameSideDataSet *dst, + const AVFrameSideDataSet src, + unsigned int flags); + /** * Get a side data entry of a specific type from a set. * diff --git a/libavutil/tests/side_data_set.c b/libavutil/tests/side_data_set.c index 056d79f655..0c9ceed962 100644 --- a/libavutil/tests/side_data_set.c +++ b/libavutil/tests/side_data_set.c @@ -91,6 +91,22 @@ int main(void) puts("\nFinal state after a single 'no-duplicates' addition:"); print_clls(set); + { + AVFrameSideDataSet dst_set = { 0 }; + av_assert0(av_frame_side_data_set_extend(&dst_set, set, 0) >= 0); + + puts("\nState of the copied set:"); + print_clls(dst_set); + + av_frame_side_data_set_uninit(&dst_set); + } + + { + int ret = av_frame_side_data_set_extend(&set, set, 0); + printf("\nResult of trying to extend a set by itself: %s\n", + av_err2str(ret)); + } + av_frame_side_data_set_uninit(&set); return 0; diff --git a/tests/ref/fate/side_data_set b/tests/ref/fate/side_data_set index 7d8c684d8f..3050b31014 100644 --- a/tests/ref/fate/side_data_set +++ b/tests/ref/fate/side_data_set @@ -12,3 +12,10 @@ Final state after a single 'no-duplicates' addition: sd 0, Ambient viewing environment sd 1, Spherical Mapping sd 2, Content light level metadata: MaxCLL: 1337 + +State of the copied set: +sd 0, Ambient viewing environment +sd 1, Spherical Mapping +sd 2, Content light level metadata: MaxCLL: 1337 + +Result of trying to extend a set by itself: Invalid argument