diff mbox series

[FFmpeg-devel,06/42] avcodec/vp8: Use RefStruct API for seg_map

Message ID AS8P250MB0744DE33C24FE20610FB3F918FFAA@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State Accepted
Commit e1ba00ac8f755f37ebc8448d3dbea906d7b79da2
Headers show
Series New API for reference counting and ThreadFrames | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Sept. 19, 2023, 7:56 p.m. UTC 
Avoids allocations and error checks when syncing the buffers.
Also avoids indirections.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/vp8.c | 23 +++++++++--------------
 libavcodec/vp8.h |  2 +-
 2 files changed, 10 insertions(+), 15 deletions(-)

Comments

Anton Khirnov Oct. 2, 2023, 9:44 a.m. UTC | #1 
Quoting Andreas Rheinhardt (2023-09-19 21:56:58)
> @@ -139,11 +138,7 @@ static int vp8_ref_frame(VP8Context *s, VP8Frame *dst, const VP8Frame *src)
>  
>      if ((ret = ff_thread_ref_frame(&dst->tf, &src->tf)) < 0)
>          return ret;
> -    if (src->seg_map &&
> -        !(dst->seg_map = av_buffer_ref(src->seg_map))) {
> -        vp8_release_frame(s, dst);
> -        return AVERROR(ENOMEM);
> -    }
> +    ff_refstruct_replace(&dst->seg_map, src->seg_map);

It seems misleading to use replace rather than ref when dst is clean at
this point.

Otherwise looks ok.
Andreas Rheinhardt Oct. 2, 2023, 10:04 a.m. UTC | #2 
Anton Khirnov:
> Quoting Andreas Rheinhardt (2023-09-19 21:56:58)
>> @@ -139,11 +138,7 @@ static int vp8_ref_frame(VP8Context *s, VP8Frame *dst, const VP8Frame *src)
>>  
>>      if ((ret = ff_thread_ref_frame(&dst->tf, &src->tf)) < 0)
>>          return ret;
>> -    if (src->seg_map &&
>> -        !(dst->seg_map = av_buffer_ref(src->seg_map))) {
>> -        vp8_release_frame(s, dst);
>> -        return AVERROR(ENOMEM);
>> -    }
>> +    ff_refstruct_replace(&dst->seg_map, src->seg_map);
> 
> It seems misleading to use replace rather than ref when dst is clean at
> this point.
> 

This is done with an eye towards 37/42, in which vp8_ref_frame() will be
replaced by vp8_replace_frame().

> Otherwise looks ok.
>
Anton Khirnov Oct. 2, 2023, 10:14 a.m. UTC | #3 
Quoting Andreas Rheinhardt (2023-10-02 12:04:02)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2023-09-19 21:56:58)
> >> @@ -139,11 +138,7 @@ static int vp8_ref_frame(VP8Context *s, VP8Frame *dst, const VP8Frame *src)
> >>  
> >>      if ((ret = ff_thread_ref_frame(&dst->tf, &src->tf)) < 0)
> >>          return ret;
> >> -    if (src->seg_map &&
> >> -        !(dst->seg_map = av_buffer_ref(src->seg_map))) {
> >> -        vp8_release_frame(s, dst);
> >> -        return AVERROR(ENOMEM);
> >> -    }
> >> +    ff_refstruct_replace(&dst->seg_map, src->seg_map);
> > 
> > It seems misleading to use replace rather than ref when dst is clean at
> > this point.
> > 
> 
> This is done with an eye towards 37/42, in which vp8_ref_frame() will be
> replaced by vp8_replace_frame().

ah, nevermind then
diff mbox series

Patch

diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index 64b1c7f60e..db325dc90b 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -34,6 +34,7 @@ 
 #include "hwaccel_internal.h"
 #include "hwconfig.h"
 #include "mathops.h"
+#include "refstruct.h"
 #include "thread.h"
 #include "threadframe.h"
 #include "vp8.h"
@@ -105,10 +106,8 @@  static int vp8_alloc_frame(VP8Context *s, VP8Frame *f, int ref)
     if ((ret = ff_thread_get_ext_buffer(s->avctx, &f->tf,
                                         ref ? AV_GET_BUFFER_FLAG_REF : 0)) < 0)
         return ret;
-    if (!(f->seg_map = av_buffer_allocz(s->mb_width * s->mb_height))) {
-        ret = AVERROR(ENOMEM);
+    if (!(f->seg_map = ff_refstruct_allocz(s->mb_width * s->mb_height)))
         goto fail;
-    }
     ret = ff_hwaccel_frame_priv_alloc(s->avctx, &f->hwaccel_picture_private,
                                       &f->hwaccel_priv_buf);
     if (ret < 0)
@@ -117,14 +116,14 @@  static int vp8_alloc_frame(VP8Context *s, VP8Frame *f, int ref)
     return 0;
 
 fail:
-    av_buffer_unref(&f->seg_map);
+    ff_refstruct_unref(&f->seg_map);
     ff_thread_release_ext_buffer(s->avctx, &f->tf);
     return ret;
 }
 
 static void vp8_release_frame(VP8Context *s, VP8Frame *f)
 {
-    av_buffer_unref(&f->seg_map);
+    ff_refstruct_unref(&f->seg_map);
     av_buffer_unref(&f->hwaccel_priv_buf);
     f->hwaccel_picture_private = NULL;
     ff_thread_release_ext_buffer(s->avctx, &f->tf);
@@ -139,11 +138,7 @@  static int vp8_ref_frame(VP8Context *s, VP8Frame *dst, const VP8Frame *src)
 
     if ((ret = ff_thread_ref_frame(&dst->tf, &src->tf)) < 0)
         return ret;
-    if (src->seg_map &&
-        !(dst->seg_map = av_buffer_ref(src->seg_map))) {
-        vp8_release_frame(s, dst);
-        return AVERROR(ENOMEM);
-    }
+    ff_refstruct_replace(&dst->seg_map, src->seg_map);
     if (src->hwaccel_picture_private) {
         dst->hwaccel_priv_buf = av_buffer_ref(src->hwaccel_priv_buf);
         if (!dst->hwaccel_priv_buf)
@@ -2334,9 +2329,9 @@  int vp78_decode_mv_mb_modes(AVCodecContext *avctx, VP8Frame *curframe,
             if (mb_y == 0)
                 AV_WN32A((mb - s->mb_width - 1)->intra4x4_pred_mode_top,
                          DC_PRED * 0x01010101);
-            decode_mb_mode(s, &s->mv_bounds, mb, mb_x, mb_y, curframe->seg_map->data + mb_xy,
+            decode_mb_mode(s, &s->mv_bounds, mb, mb_x, mb_y, curframe->seg_map + mb_xy,
                            prev_frame && prev_frame->seg_map ?
-                           prev_frame->seg_map->data + mb_xy : NULL, 1, is_vp7);
+                           prev_frame->seg_map + mb_xy : NULL, 1, is_vp7);
             s->mv_bounds.mv_min.x -= 64;
             s->mv_bounds.mv_max.x -= 64;
         }
@@ -2467,9 +2462,9 @@  static av_always_inline int decode_mb_row_no_filter(AVCodecContext *avctx, void
                          dst[2] - dst[1], 2);
 
         if (!s->mb_layout)
-            decode_mb_mode(s, &td->mv_bounds, mb, mb_x, mb_y, curframe->seg_map->data + mb_xy,
+            decode_mb_mode(s, &td->mv_bounds, mb, mb_x, mb_y, curframe->seg_map + mb_xy,
                            prev_frame && prev_frame->seg_map ?
-                           prev_frame->seg_map->data + mb_xy : NULL, 0, is_vp7);
+                           prev_frame->seg_map + mb_xy : NULL, 0, is_vp7);
 
         prefetch_motion(s, mb, mb_x, mb_y, mb_xy, VP8_FRAME_PREVIOUS);
 
diff --git a/libavcodec/vp8.h b/libavcodec/vp8.h
index 6f29156b53..cb752d4498 100644
--- a/libavcodec/vp8.h
+++ b/libavcodec/vp8.h
@@ -152,7 +152,7 @@  typedef struct VP8ThreadData {
 
 typedef struct VP8Frame {
     ThreadFrame tf;
-    AVBufferRef *seg_map;
+    uint8_t *seg_map; ///< RefStruct reference
 
     AVBufferRef *hwaccel_priv_buf;
     void *hwaccel_picture_private;