On 6/7/2022 7:21 AM, Anton Khirnov wrote:
Quoting James Almer (2022-06-05 19:44:58)
+
+ /* duplicate the frame data if it's not refcounted */
+ if (!src->buf[0]) {
+ for (int i = 0; i < FF_ARRAY_ELEMS(dst->buf); i++)
+ av_buffer_unref(&dst->buf[i]);
+ for (int 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;
This looks suspicious, since we just allocated normal buffers for dst.
Right, removed.
Also, can't t this whole block be replaced by just
if (!src->buf[0]) {
av_frame_unref(dst);
return av_frame_ref(dst, src);
}
at the beginning of this function?
Yes, but i was copying how av_frame_ref() handled this scenario, albeit
with a bit more of code.
Will change, but it will need to be re-added if someone at some point
decides to implement replace for side 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);
nit: av_memdup()?
+ } 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..e5c10e2b66 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -752,6 +752,19 @@ 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.
+ *
+ * @return 0 on success, a negative AVERROR on error. On error, dst is
+ * unreferenced.
+ */
+int av_frame_replace(AVFrame *dst, const AVFrame *src);
An important property of av_buffer_replace() is that it's equivalent to
av_buffer_unref(dst) when src is NULL. It would probably be desirable
for av_frame_replace() to work the same way - currently it will try to
call av_frame_get_buffer() with invalid parameters and fail.
Should it really accept NULL as src, or you meant a recently
allocated/unreff'd frame as src (So src->data[0] == NULL)?
_______________________________________________
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".
