On 8/8/2021 9:34 PM, Andreas Rheinhardt wrote:
James Almer:
On 8/8/2021 8:16 PM, Andreas Rheinhardt wrote:
James Almer:
Will remove unnecessary allocations when both src and dst picture
contain
references to the same buffers.
Signed-off-by: James Almer <jamr...@gmail.com>
---
libavcodec/h264_picture.c | 45 +++++++++++++++++++++++++++++++++++++++
libavcodec/h264dec.h | 1 +
2 files changed, 46 insertions(+)
diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c
index 89aef37edd..1073d9e7e0 100644
--- a/libavcodec/h264_picture.c
+++ b/libavcodec/h264_picture.c
@@ -142,6 +142,51 @@ fail:
return ret;
}
+int ff_h264_replace_picture(H264Context *h, H264Picture *dst,
H264Picture *src)
Is there something that hinders you from making src const? If so, I
don't see it.
Amended locally (Also h264_copy_picture_params() in patch 1/3).
+{
+ int ret, i;
+
+ if (!src->f || !src->f->buf[0]) {
+ ff_h264_unref_picture(h, dst);
+ return 0;
+ }
+
+ av_assert0(src->tf.f == src->f);
+
+ dst->tf.f = dst->f;
+ ff_thread_release_buffer(h->avctx, &dst->tf);
+ ret = ff_thread_ref_frame(&dst->tf, &src->tf);
+ if (ret < 0)
+ goto fail;
+
+ ret = av_buffer_replace(&dst->qscale_table_buf,
src->qscale_table_buf);
+ ret |= av_buffer_replace(&dst->mb_type_buf, src->mb_type_buf);
+ ret |= av_buffer_replace(&dst->pps_buf, src->pps_buf);
+ if (ret < 0)
+ goto fail;
+
+ for (i = 0; i < 2; i++) {
+ ret = av_buffer_replace(&dst->motion_val_buf[i],
src->motion_val_buf[i]);
+ ret |= av_buffer_replace(&dst->ref_index_buf[i],
src->ref_index_buf[i]);
+ if (ret < 0)
+ goto fail;
+ }
+
+ if (src->hwaccel_picture_private) {
dst in this function is allowed to be used/dirty; so I don't see
anything that precludes dst->hwaccel_picture_private/hwaccel_priv_buf to
be set while the same is not true for src. But then this check means
that dst is not equivalent to src.
+ ret = av_buffer_replace(&dst->hwaccel_priv_buf,
src->hwaccel_priv_buf);
+ if (ret < 0)
+ goto fail;
+ dst->hwaccel_picture_private = dst->hwaccel_priv_buf->data;
This is the only thing that needs to be under if.
Amended locally into:
ret = av_buffer_replace(&dst->hwaccel_priv_buf,
src->hwaccel_priv_buf);
if (ret < 0)
goto fail;
dst->hwaccel_picture_private = NULL;
if (src->hwaccel_picture_private)
dst->hwaccel_picture_private = dst->hwaccel_priv_buf->data;
On second look wouldn't dst->hwaccel_picture_private =
src->hwaccel_picture_private be the same thing, but without a branch?
Yeah, I'll remove the branch and just copy the field.
(And even if it could happen (I doubt it) that
src->hwaccel_picture_private and src->hwaccel_priv_buf->data differ then
shouldn't we set the hwaccel_picture_private from src's
hwaccel_picture_private to make src and dst equivalent?)
If they could differ, then ff_h264_ref_picture() would be broken. So
yeah, like i said above I'll just copy the field.
- Andreas
_______________________________________________
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".
_______________________________________________
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".
