| Message ID | 20220605152813.1888-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avutil/frame: add av_frame_replace | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
James Almer: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > Now taking into account the new channel layout API fields, supporting non > refcouted src, plus changing the documentation to state that the dst frame > properties will be replaced by those from src even if the data described > by both frames is the same. > Apparently, you didn't get my point last time: What happens if dst and src coincide did not mean "what happpens if the data both frames refer to already coincides"; it meant "what happens if src == dst?" (so that there is only one AVFrame involved). You either disallow this altogether or you support it. This patch (and the last one) does neither. > Still missing APIChanges entry and version bump. > > libavutil/frame.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++ > libavutil/frame.h | 17 ++++++ > 2 files changed, 161 insertions(+) > > diff --git a/libavutil/frame.c b/libavutil/frame.c > index 4c16488c66..2a4a2c27ad 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -461,6 +461,150 @@ fail: > return ret; > } > > +int av_frame_replace(AVFrame *dst, const AVFrame *src) > +{ > + int i, ret = 0; Why don't you use "for (int i = 0;" loop variables? > + > + dst->format = src->format; > + dst->width = src->width; > + dst->height = src->height; > + dst->nb_samples = src->nb_samples; > +#if FF_API_OLD_CHANNEL_LAYOUT > +FF_DISABLE_DEPRECATION_WARNINGS > + dst->channels = src->channels; > + dst->channel_layout = src->channel_layout; > + if (!av_channel_layout_check(&src->ch_layout)) { > + av_channel_layout_uninit(&dst->ch_layout); > + if (src->channel_layout) > + av_channel_layout_from_mask(&dst->ch_layout, src->channel_layout); > + else { > + dst->ch_layout.nb_channels = src->channels; > + dst->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; > + } > + } else { > +#endif > + ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout); > + if (ret < 0) > + goto fail; > +#if FF_API_OLD_CHANNEL_LAYOUT > + } > +FF_ENABLE_DEPRECATION_WARNINGS > +#endif > + > + wipe_side_data(dst); > + av_dict_free(&dst->metadata); > + ret = frame_copy_props(dst, src, 0); > + if (ret < 0) > + goto fail; > + > + /* duplicate the frame data if it's not refcounted */ > + if (!src->buf[0]) { > + for (i = 0; i < FF_ARRAY_ELEMS(dst->buf); i++) > + av_buffer_unref(&dst->buf[i]); > + for (i = 0; i < dst->nb_extended_buf; i++) > + av_buffer_unref(&dst->extended_buf[i]); > + av_freep(&dst->extended_buf); > + > + memset(dst->data, 0, sizeof(dst->data)); > + if (dst->extended_data != dst->data) > + av_freep(&dst->extended_data); > + > + ret = av_frame_get_buffer(dst, 0); > + if (ret < 0) > + goto fail; > + > + ret = av_frame_copy(dst, src); > + if (ret < 0) > + goto fail; > + > + ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx); > + if (ret < 0) > + goto fail; > + > + return 0; > + } > + > + /* replace the buffers */ > + for (i = 0; i < FF_ARRAY_ELEMS(src->buf); i++) { > + ret = av_buffer_replace(&dst->buf[i], src->buf[i]); > + if (ret < 0) > + goto fail; > + } > + > + if (src->extended_buf) { > + if (dst->nb_extended_buf != src->nb_extended_buf) { > + int nb_extended_buf = FFMIN(dst->nb_extended_buf, src->nb_extended_buf); > + void *tmp; > + > + for (i = nb_extended_buf; i < dst->nb_extended_buf; i++) > + av_buffer_unref(&dst->extended_buf[i]); > + > + tmp = av_realloc_array(dst->extended_buf, sizeof(*dst->extended_buf), > + src->nb_extended_buf); > + if (!tmp) { > + ret = AVERROR(ENOMEM); > + goto fail; > + } > + dst->extended_buf = tmp; > + dst->nb_extended_buf = src->nb_extended_buf; > + > + memset(&dst->extended_buf[nb_extended_buf], 0, > + (src->nb_extended_buf - nb_extended_buf) * sizeof(*dst->extended_buf)); > + } > + > + for (i = 0; i < src->nb_extended_buf; i++) { > + ret = av_buffer_replace(&dst->extended_buf[i], src->extended_buf[i]); > + if (ret < 0) > + goto fail; > + } > + } else if (dst->extended_buf) { > + for (i = 0; i < dst->nb_extended_buf; i++) > + av_buffer_unref(&dst->extended_buf[i]); > + av_freep(&dst->extended_buf); > + } > + > + ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx); > + if (ret < 0) > + goto fail; > + > + if (dst->extended_data != dst->data) > + av_freep(&dst->extended_data); > + > + if (src->extended_data != src->data) { > + int ch = src->ch_layout.nb_channels; > + > +#if FF_API_OLD_CHANNEL_LAYOUT > +FF_DISABLE_DEPRECATION_WARNINGS > + if (!ch) { > + ch = src->channels; > + CHECK_CHANNELS_CONSISTENCY(src); > + } > +FF_ENABLE_DEPRECATION_WARNINGS > +#endif > + if (!ch) { > + ret = AVERROR(EINVAL); > + goto fail; > + } > + > + dst->extended_data = av_malloc_array(sizeof(*dst->extended_data), ch); > + if (!dst->extended_data) { > + ret = AVERROR(ENOMEM); > + goto fail; > + } > + memcpy(dst->extended_data, src->extended_data, sizeof(*src->extended_data) * ch); > + } else > + dst->extended_data = dst->data; > + > + memcpy(dst->data, src->data, sizeof(src->data)); > + memcpy(dst->linesize, src->linesize, sizeof(src->linesize)); > + > + return 0; > + > +fail: > + av_frame_unref(dst); > + return ret; > +} > + > AVFrame *av_frame_clone(const AVFrame *src) > { > AVFrame *ret = av_frame_alloc(); > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 33fac2054c..a971b1655e 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -752,6 +752,23 @@ void av_frame_free(AVFrame **frame); > */ > int av_frame_ref(AVFrame *dst, const AVFrame *src); > > +/** > + * Ensure the destination frame refers to the same data described by the source > + * frame by creating a new reference for each AVBufferRef from src if they > + * differ from those in dst, or if src is not reference counted, by allocating > + * new buffers and copying data. > + * > + * Frame properties on dst will be replaced by those from src. > + * > + * @param src The source frame. If there's data described in it, it must be > + * reference counted. The text above says "if src is not reference counted, by allocating new buffers and copying data". > + * @param dst The destination frame. > + * > + * @return 0 on success, a negative AVERROR on error. On error, dst is > + * unreferenced. > + */ > +int av_frame_replace(AVFrame *dst, const AVFrame *src); > + > /** > * Create a new frame that references the same data as src. > *
On 6/5/2022 12:42 PM, Andreas Rheinhardt wrote: > James Almer: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> Now taking into account the new channel layout API fields, supporting non >> refcouted src, plus changing the documentation to state that the dst frame >> properties will be replaced by those from src even if the data described >> by both frames is the same. >> > > Apparently, you didn't get my point last time: What happens if dst and > src coincide did not mean "what happpens if the data both frames refer > to already coincides"; it meant "what happens if src == dst?" (so that > there is only one AVFrame involved). You either disallow this altogether > or you support it. This patch (and the last one) does neither. Ok, will disallow this with if (dst == src) return AVERROR(EINVAL); How does av_frame_ref() handle this scenario, for that matter? The checks i see to ensure dst is clean (and thus one could assume is different than src) are non strict with av_assert1(). > >> Still missing APIChanges entry and version bump. >> >> libavutil/frame.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++ >> libavutil/frame.h | 17 ++++++ >> 2 files changed, 161 insertions(+) >> >> diff --git a/libavutil/frame.c b/libavutil/frame.c >> index 4c16488c66..2a4a2c27ad 100644 >> --- a/libavutil/frame.c >> +++ b/libavutil/frame.c >> @@ -461,6 +461,150 @@ fail: >> return ret; >> } >> >> +int av_frame_replace(AVFrame *dst, const AVFrame *src) >> +{ >> + int i, ret = 0; > > Why don't you use "for (int i = 0;" loop variables? Because i copied this from av_frame_ref(). Will change it. > >> + >> + dst->format = src->format; >> + dst->width = src->width; >> + dst->height = src->height; >> + dst->nb_samples = src->nb_samples; >> +#if FF_API_OLD_CHANNEL_LAYOUT >> +FF_DISABLE_DEPRECATION_WARNINGS >> + dst->channels = src->channels; >> + dst->channel_layout = src->channel_layout; >> + if (!av_channel_layout_check(&src->ch_layout)) { >> + av_channel_layout_uninit(&dst->ch_layout); >> + if (src->channel_layout) >> + av_channel_layout_from_mask(&dst->ch_layout, src->channel_layout); >> + else { >> + dst->ch_layout.nb_channels = src->channels; >> + dst->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; >> + } >> + } else { >> +#endif >> + ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout); >> + if (ret < 0) >> + goto fail; >> +#if FF_API_OLD_CHANNEL_LAYOUT >> + } >> +FF_ENABLE_DEPRECATION_WARNINGS >> +#endif >> + >> + wipe_side_data(dst); >> + av_dict_free(&dst->metadata); >> + ret = frame_copy_props(dst, src, 0); >> + if (ret < 0) >> + goto fail; >> + >> + /* duplicate the frame data if it's not refcounted */ >> + if (!src->buf[0]) { >> + for (i = 0; i < FF_ARRAY_ELEMS(dst->buf); i++) >> + av_buffer_unref(&dst->buf[i]); >> + for (i = 0; i < dst->nb_extended_buf; i++) >> + av_buffer_unref(&dst->extended_buf[i]); >> + av_freep(&dst->extended_buf); >> + >> + memset(dst->data, 0, sizeof(dst->data)); >> + if (dst->extended_data != dst->data) >> + av_freep(&dst->extended_data); >> + >> + ret = av_frame_get_buffer(dst, 0); >> + if (ret < 0) >> + goto fail; >> + >> + ret = av_frame_copy(dst, src); >> + if (ret < 0) >> + goto fail; >> + >> + ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx); >> + if (ret < 0) >> + goto fail; >> + >> + return 0; >> + } >> + >> + /* replace the buffers */ >> + for (i = 0; i < FF_ARRAY_ELEMS(src->buf); i++) { >> + ret = av_buffer_replace(&dst->buf[i], src->buf[i]); >> + if (ret < 0) >> + goto fail; >> + } >> + >> + if (src->extended_buf) { >> + if (dst->nb_extended_buf != src->nb_extended_buf) { >> + int nb_extended_buf = FFMIN(dst->nb_extended_buf, src->nb_extended_buf); >> + void *tmp; >> + >> + for (i = nb_extended_buf; i < dst->nb_extended_buf; i++) >> + av_buffer_unref(&dst->extended_buf[i]); >> + >> + tmp = av_realloc_array(dst->extended_buf, sizeof(*dst->extended_buf), >> + src->nb_extended_buf); >> + if (!tmp) { >> + ret = AVERROR(ENOMEM); >> + goto fail; >> + } >> + dst->extended_buf = tmp; >> + dst->nb_extended_buf = src->nb_extended_buf; >> + >> + memset(&dst->extended_buf[nb_extended_buf], 0, >> + (src->nb_extended_buf - nb_extended_buf) * sizeof(*dst->extended_buf)); >> + } >> + >> + for (i = 0; i < src->nb_extended_buf; i++) { >> + ret = av_buffer_replace(&dst->extended_buf[i], src->extended_buf[i]); >> + if (ret < 0) >> + goto fail; >> + } >> + } else if (dst->extended_buf) { >> + for (i = 0; i < dst->nb_extended_buf; i++) >> + av_buffer_unref(&dst->extended_buf[i]); >> + av_freep(&dst->extended_buf); >> + } >> + >> + ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx); >> + if (ret < 0) >> + goto fail; >> + >> + if (dst->extended_data != dst->data) >> + av_freep(&dst->extended_data); >> + >> + if (src->extended_data != src->data) { >> + int ch = src->ch_layout.nb_channels; >> + >> +#if FF_API_OLD_CHANNEL_LAYOUT >> +FF_DISABLE_DEPRECATION_WARNINGS >> + if (!ch) { >> + ch = src->channels; >> + CHECK_CHANNELS_CONSISTENCY(src); >> + } >> +FF_ENABLE_DEPRECATION_WARNINGS >> +#endif >> + if (!ch) { >> + ret = AVERROR(EINVAL); >> + goto fail; >> + } >> + >> + dst->extended_data = av_malloc_array(sizeof(*dst->extended_data), ch); >> + if (!dst->extended_data) { >> + ret = AVERROR(ENOMEM); >> + goto fail; >> + } >> + memcpy(dst->extended_data, src->extended_data, sizeof(*src->extended_data) * ch); >> + } else >> + dst->extended_data = dst->data; >> + >> + memcpy(dst->data, src->data, sizeof(src->data)); >> + memcpy(dst->linesize, src->linesize, sizeof(src->linesize)); >> + >> + return 0; >> + >> +fail: >> + av_frame_unref(dst); >> + return ret; >> +} >> + >> AVFrame *av_frame_clone(const AVFrame *src) >> { >> AVFrame *ret = av_frame_alloc(); >> diff --git a/libavutil/frame.h b/libavutil/frame.h >> index 33fac2054c..a971b1655e 100644 >> --- a/libavutil/frame.h >> +++ b/libavutil/frame.h >> @@ -752,6 +752,23 @@ void av_frame_free(AVFrame **frame); >> */ >> int av_frame_ref(AVFrame *dst, const AVFrame *src); >> >> +/** >> + * Ensure the destination frame refers to the same data described by the source >> + * frame by creating a new reference for each AVBufferRef from src if they >> + * differ from those in dst, or if src is not reference counted, by allocating >> + * new buffers and copying data. >> + * >> + * Frame properties on dst will be replaced by those from src. >> + * >> + * @param src The source frame. If there's data described in it, it must be >> + * reference counted. > > The text above says "if src is not reference counted, by allocating new > buffers and copying data". Remnant from the previous version. Will fix. > >> + * @param dst The destination frame. >> + * >> + * @return 0 on success, a negative AVERROR on error. On error, dst is >> + * unreferenced. >> + */ >> +int av_frame_replace(AVFrame *dst, const AVFrame *src); >> + >> /** >> * Create a new frame that references the same data as src. >> * > > _______________________________________________ > 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".
diff --git a/libavutil/frame.c b/libavutil/frame.c index 4c16488c66..2a4a2c27ad 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -461,6 +461,150 @@ fail: return ret; } +int av_frame_replace(AVFrame *dst, const AVFrame *src) +{ + int i, ret = 0; + + dst->format = src->format; + dst->width = src->width; + dst->height = src->height; + dst->nb_samples = src->nb_samples; +#if FF_API_OLD_CHANNEL_LAYOUT +FF_DISABLE_DEPRECATION_WARNINGS + dst->channels = src->channels; + dst->channel_layout = src->channel_layout; + if (!av_channel_layout_check(&src->ch_layout)) { + av_channel_layout_uninit(&dst->ch_layout); + if (src->channel_layout) + av_channel_layout_from_mask(&dst->ch_layout, src->channel_layout); + else { + dst->ch_layout.nb_channels = src->channels; + dst->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; + } + } else { +#endif + ret = av_channel_layout_copy(&dst->ch_layout, &src->ch_layout); + if (ret < 0) + goto fail; +#if FF_API_OLD_CHANNEL_LAYOUT + } +FF_ENABLE_DEPRECATION_WARNINGS +#endif + + wipe_side_data(dst); + av_dict_free(&dst->metadata); + ret = frame_copy_props(dst, src, 0); + if (ret < 0) + goto fail; + + /* duplicate the frame data if it's not refcounted */ + if (!src->buf[0]) { + for (i = 0; i < FF_ARRAY_ELEMS(dst->buf); i++) + av_buffer_unref(&dst->buf[i]); + for (i = 0; i < dst->nb_extended_buf; i++) + av_buffer_unref(&dst->extended_buf[i]); + av_freep(&dst->extended_buf); + + memset(dst->data, 0, sizeof(dst->data)); + if (dst->extended_data != dst->data) + av_freep(&dst->extended_data); + + ret = av_frame_get_buffer(dst, 0); + if (ret < 0) + goto fail; + + ret = av_frame_copy(dst, src); + if (ret < 0) + goto fail; + + ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx); + if (ret < 0) + goto fail; + + return 0; + } + + /* replace the buffers */ + for (i = 0; i < FF_ARRAY_ELEMS(src->buf); i++) { + ret = av_buffer_replace(&dst->buf[i], src->buf[i]); + if (ret < 0) + goto fail; + } + + if (src->extended_buf) { + if (dst->nb_extended_buf != src->nb_extended_buf) { + int nb_extended_buf = FFMIN(dst->nb_extended_buf, src->nb_extended_buf); + void *tmp; + + for (i = nb_extended_buf; i < dst->nb_extended_buf; i++) + av_buffer_unref(&dst->extended_buf[i]); + + tmp = av_realloc_array(dst->extended_buf, sizeof(*dst->extended_buf), + src->nb_extended_buf); + if (!tmp) { + ret = AVERROR(ENOMEM); + goto fail; + } + dst->extended_buf = tmp; + dst->nb_extended_buf = src->nb_extended_buf; + + memset(&dst->extended_buf[nb_extended_buf], 0, + (src->nb_extended_buf - nb_extended_buf) * sizeof(*dst->extended_buf)); + } + + for (i = 0; i < src->nb_extended_buf; i++) { + ret = av_buffer_replace(&dst->extended_buf[i], src->extended_buf[i]); + if (ret < 0) + goto fail; + } + } else if (dst->extended_buf) { + for (i = 0; i < dst->nb_extended_buf; i++) + av_buffer_unref(&dst->extended_buf[i]); + av_freep(&dst->extended_buf); + } + + ret = av_buffer_replace(&dst->hw_frames_ctx, src->hw_frames_ctx); + if (ret < 0) + goto fail; + + if (dst->extended_data != dst->data) + av_freep(&dst->extended_data); + + if (src->extended_data != src->data) { + int ch = src->ch_layout.nb_channels; + +#if FF_API_OLD_CHANNEL_LAYOUT +FF_DISABLE_DEPRECATION_WARNINGS + if (!ch) { + ch = src->channels; + CHECK_CHANNELS_CONSISTENCY(src); + } +FF_ENABLE_DEPRECATION_WARNINGS +#endif + if (!ch) { + ret = AVERROR(EINVAL); + goto fail; + } + + dst->extended_data = av_malloc_array(sizeof(*dst->extended_data), ch); + if (!dst->extended_data) { + ret = AVERROR(ENOMEM); + goto fail; + } + memcpy(dst->extended_data, src->extended_data, sizeof(*src->extended_data) * ch); + } else + dst->extended_data = dst->data; + + memcpy(dst->data, src->data, sizeof(src->data)); + memcpy(dst->linesize, src->linesize, sizeof(src->linesize)); + + return 0; + +fail: + av_frame_unref(dst); + return ret; +} + AVFrame *av_frame_clone(const AVFrame *src) { AVFrame *ret = av_frame_alloc(); diff --git a/libavutil/frame.h b/libavutil/frame.h index 33fac2054c..a971b1655e 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -752,6 +752,23 @@ void av_frame_free(AVFrame **frame); */ int av_frame_ref(AVFrame *dst, const AVFrame *src); +/** + * Ensure the destination frame refers to the same data described by the source + * frame by creating a new reference for each AVBufferRef from src if they + * differ from those in dst, or if src is not reference counted, by allocating + * new buffers and copying data. + * + * Frame properties on dst will be replaced by those from src. + * + * @param src The source frame. If there's data described in it, it must be + * reference counted. + * @param dst The destination frame. + * + * @return 0 on success, a negative AVERROR on error. On error, dst is + * unreferenced. + */ +int av_frame_replace(AVFrame *dst, const AVFrame *src); + /** * Create a new frame that references the same data as src. *
Signed-off-by: James Almer <jamrial@gmail.com> --- Now taking into account the new channel layout API fields, supporting non refcouted src, plus changing the documentation to state that the dst frame properties will be replaced by those from src even if the data described by both frames is the same. Still missing APIChanges entry and version bump. libavutil/frame.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++ libavutil/frame.h | 17 ++++++ 2 files changed, 161 insertions(+)