| Message ID | 20210808225612.11600-3-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | 90bf83d6f1fb04a0dea7d87e536ea7cb25fed21b |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/h264_picture: split copying some H264Picture fields into a separate function | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
James Almer: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/h264_slice.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c > index a31e804620..0d7107d455 100644 > --- a/libavcodec/h264_slice.c > +++ b/libavcodec/h264_slice.c > @@ -379,19 +379,15 @@ int ff_h264_update_thread_context(AVCodecContext *dst, > h->droppable = h1->droppable; > > for (i = 0; i < H264_MAX_PICTURE_COUNT; i++) { > - ff_h264_unref_picture(h, &h->DPB[i]); > - if (h1->DPB[i].f->buf[0] && > - (ret = ff_h264_ref_picture(h, &h->DPB[i], &h1->DPB[i])) < 0) > + ret = ff_h264_replace_picture(h, &h->DPB[i], &h1->DPB[i]); > + if (ret < 0) > return ret; > } > > h->cur_pic_ptr = REBASE_PICTURE(h1->cur_pic_ptr, h, h1); > - ff_h264_unref_picture(h, &h->cur_pic); > - if (h1->cur_pic.f->buf[0]) { > - ret = ff_h264_ref_picture(h, &h->cur_pic, &h1->cur_pic); > - if (ret < 0) > - return ret; > - } > + ret = ff_h264_replace_picture(h, &h->cur_pic, &h1->cur_pic); > + if (ret < 0) > + return ret; > > h->enable_er = h1->enable_er; > h->workaround_bugs = h1->workaround_bugs; > I like what you are doing here; I once had the same idea, but IIRC it was way uglier. Can you share some numbers on how many allocations are avoided by this? (I guess that avoiding the allocation will be the normal case for at least the PPS buffer.) How many would it be with this patch: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272508.html? - Andreas
On 8/8/2021 8:22 PM, Andreas Rheinhardt wrote: > James Almer: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/h264_slice.c | 14 +++++--------- >> 1 file changed, 5 insertions(+), 9 deletions(-) >> >> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c >> index a31e804620..0d7107d455 100644 >> --- a/libavcodec/h264_slice.c >> +++ b/libavcodec/h264_slice.c >> @@ -379,19 +379,15 @@ int ff_h264_update_thread_context(AVCodecContext *dst, >> h->droppable = h1->droppable; >> >> for (i = 0; i < H264_MAX_PICTURE_COUNT; i++) { >> - ff_h264_unref_picture(h, &h->DPB[i]); >> - if (h1->DPB[i].f->buf[0] && >> - (ret = ff_h264_ref_picture(h, &h->DPB[i], &h1->DPB[i])) < 0) >> + ret = ff_h264_replace_picture(h, &h->DPB[i], &h1->DPB[i]); >> + if (ret < 0) >> return ret; >> } >> >> h->cur_pic_ptr = REBASE_PICTURE(h1->cur_pic_ptr, h, h1); >> - ff_h264_unref_picture(h, &h->cur_pic); >> - if (h1->cur_pic.f->buf[0]) { >> - ret = ff_h264_ref_picture(h, &h->cur_pic, &h1->cur_pic); >> - if (ret < 0) >> - return ret; >> - } >> + ret = ff_h264_replace_picture(h, &h->cur_pic, &h1->cur_pic); >> + if (ret < 0) >> + return ret; >> >> h->enable_er = h1->enable_er; >> h->workaround_bugs = h1->workaround_bugs; >> > I like what you are doing here; I once had the same idea, but IIRC it > was way uglier. > Can you share some numbers on how many allocations are avoided by this? I did not look into what specific buffers within H264Picture were not allocated (unref + ref) after this patch, but i did check what calls to ff_h264_replace_picture() had to do it for at least one of them, and in a -threads 4 scenario for a standard 1080p high profile sample cur_pic pretty much always generated allocations, but only about three out of all DPB pictures (about sixteen) did it. Considering the above, it would be nice if we could do the same for the AVFrame too, but when i looked at adding an av_frame_replace() it looked like it wouldn't be trivial. > (I guess that avoiding the allocation will be the normal case for at > least the PPS buffer.) How many would it be with this patch: > https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272508.html? > > - 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-request@ffmpeg.org with subject "unsubscribe". >
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c index a31e804620..0d7107d455 100644 --- a/libavcodec/h264_slice.c +++ b/libavcodec/h264_slice.c @@ -379,19 +379,15 @@ int ff_h264_update_thread_context(AVCodecContext *dst, h->droppable = h1->droppable; for (i = 0; i < H264_MAX_PICTURE_COUNT; i++) { - ff_h264_unref_picture(h, &h->DPB[i]); - if (h1->DPB[i].f->buf[0] && - (ret = ff_h264_ref_picture(h, &h->DPB[i], &h1->DPB[i])) < 0) + ret = ff_h264_replace_picture(h, &h->DPB[i], &h1->DPB[i]); + if (ret < 0) return ret; } h->cur_pic_ptr = REBASE_PICTURE(h1->cur_pic_ptr, h, h1); - ff_h264_unref_picture(h, &h->cur_pic); - if (h1->cur_pic.f->buf[0]) { - ret = ff_h264_ref_picture(h, &h->cur_pic, &h1->cur_pic); - if (ret < 0) - return ret; - } + ret = ff_h264_replace_picture(h, &h->cur_pic, &h1->cur_pic); + if (ret < 0) + return ret; h->enable_er = h1->enable_er; h->workaround_bugs = h1->workaround_bugs;
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/h264_slice.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)