decoder: h264: fix frame store logic for MVC.

In strict MVC decoding mode, when only the necessary set of inter-view
reference pictures are passed to the ReferenceFrames array for decoding
the current picture, we should not re-use a frame store id that might
be needed for decoding another view component in the same access unit.

One way to solve this problem is to track when the VA surface in a
specified frame store id was last referenced. So, a "ref_age" field
is introduced to the GenFrameStore struct and is updated whenever
the surface is being referenced.

Additionally, the list of retired refs candidates (free_refs) is kept
ordered by increasing ref_age. That way, we can immediately know what
is the oldest frame store id to recycle.

Let deltaAge = CurrAge - RefAge:
If deltaAge > 1, we know for sure that the VA surface is gone ;
If deltaAge = 1, the surface could be re-used for inter prediction ;
If deltaAge = 0, the surface could be re-used for inter-view prediction.

The ref_age in each Frame Store entry is always current, i.e. it is
the same for all reference frames that intervened in the decoding
process of all inter view components of the previous access unit. The
age tracks access units.

v2: used a more correct representation of age, instead of POC [Yakui]
v3: minor optimization for detecting changes of access unit [Haihao]

Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
(cherry picked from commit 3b5eb0522fbfe1220dcd0c0bb093a93cfc25e22c)

diff --git a/src/gen6_mfd.c b/src/gen6_mfd.c
index 437ad3b..8128a80 100755 (executable)
--- a/src/gen6_mfd.c
+++ b/src/gen6_mfd.c
@@ -61,6 +61,7 @@ gen6_mfd_init_avc_surface(VADriverContextP ctx,
 
     if (!gen6_avc_surface) {
         gen6_avc_surface = calloc(sizeof(GenAvcSurface), 1);
+        gen6_avc_surface->frame_store_id = -1;
         assert((obj_surface->size & 0x3f) == 0);
         obj_surface->private_data = gen6_avc_surface;
     }
@@ -825,7 +826,8 @@ gen6_mfd_avc_decode_init(VADriverContextP ctx,
 
     assert(decode_state->pic_param && decode_state->pic_param->buffer);
     pic_param = (VAPictureParameterBufferH264 *)decode_state->pic_param->buffer;
-    intel_update_avc_frame_store_index(ctx, decode_state, pic_param, gen6_mfd_context->reference_surface);
+    intel_update_avc_frame_store_index(ctx, decode_state, pic_param,
+        gen6_mfd_context->reference_surface, &gen6_mfd_context->fs_ctx);
     width_in_mbs = ((pic_param->picture_width_in_mbs_minus1 + 1) & 0xff);
 
     /* Current decoded picture */

diff --git a/src/gen6_mfd.h b/src/gen6_mfd.h
index de131d6..f499803 100644 (file)
--- a/src/gen6_mfd.h
+++ b/src/gen6_mfd.h
@@ -62,6 +62,7 @@ struct gen6_mfd_context
         VAIQMatrixBufferMPEG2 mpeg2;
     } iq_matrix;
 
+    GenFrameStoreContext fs_ctx;
     GenFrameStore       reference_surface[MAX_GEN_REFERENCE_FRAMES];
     GenBuffer           post_deblocking_output;
     GenBuffer           pre_deblocking_output;

diff --git a/src/gen75_mfd.c b/src/gen75_mfd.c
index a1d004b..a89640d 100644 (file)
--- a/src/gen75_mfd.c
+++ b/src/gen75_mfd.c
@@ -67,6 +67,7 @@ gen75_mfd_init_avc_surface(VADriverContextP ctx,
 
     if (!gen7_avc_surface) {
         gen7_avc_surface = calloc(sizeof(GenAvcSurface), 1);
+        gen7_avc_surface->frame_store_id = -1;
         assert((obj_surface->size & 0x3f) == 0);
         obj_surface->private_data = gen7_avc_surface;
     }

diff --git a/src/gen7_mfd.c b/src/gen7_mfd.c
index 97f9705..7ab2955 100755 (executable)
--- a/src/gen7_mfd.c
+++ b/src/gen7_mfd.c
@@ -65,6 +65,7 @@ gen7_mfd_init_avc_surface(VADriverContextP ctx,
 
     if (!gen7_avc_surface) {
         gen7_avc_surface = calloc(sizeof(GenAvcSurface), 1);
+        gen7_avc_surface->frame_store_id = -1;
         assert((obj_surface->size & 0x3f) == 0);
         obj_surface->private_data = gen7_avc_surface;
     }
@@ -740,7 +741,8 @@ gen7_mfd_avc_decode_init(VADriverContextP ctx,
 
     assert(decode_state->pic_param && decode_state->pic_param->buffer);
     pic_param = (VAPictureParameterBufferH264 *)decode_state->pic_param->buffer;
-    intel_update_avc_frame_store_index(ctx, decode_state, pic_param, gen7_mfd_context->reference_surface);
+    intel_update_avc_frame_store_index(ctx, decode_state, pic_param,
+        gen7_mfd_context->reference_surface, &gen7_mfd_context->fs_ctx);
     width_in_mbs = pic_param->picture_width_in_mbs_minus1 + 1;
     height_in_mbs = pic_param->picture_height_in_mbs_minus1 + 1;
     assert(width_in_mbs > 0 && width_in_mbs <= 256); /* 4K */

diff --git a/src/gen7_mfd.h b/src/gen7_mfd.h
index 0200216..af8e960 100644 (file)
--- a/src/gen7_mfd.h
+++ b/src/gen7_mfd.h
@@ -77,6 +77,7 @@ struct gen7_mfd_context
         VAIQMatrixBufferH264  h264;     /* flat scaling lists (default) */
     } iq_matrix;
 
+    GenFrameStoreContext fs_ctx;
     GenFrameStore       reference_surface[MAX_GEN_REFERENCE_FRAMES];
     GenBuffer           post_deblocking_output;
     GenBuffer           pre_deblocking_output;

diff --git a/src/gen8_mfd.c b/src/gen8_mfd.c
index df0cd42..5e1b70b 100644 (file)
--- a/src/gen8_mfd.c
+++ b/src/gen8_mfd.c
@@ -74,6 +74,7 @@ gen8_mfd_init_avc_surface(VADriverContextP ctx,
 
     if (!gen7_avc_surface) {
         gen7_avc_surface = calloc(sizeof(GenAvcSurface), 1);
+        gen7_avc_surface->frame_store_id = -1;
         assert((obj_surface->size & 0x3f) == 0);
         obj_surface->private_data = gen7_avc_surface;
     }

diff --git a/src/i965_avc_bsd.c b/src/i965_avc_bsd.c
index aca3c01..ebeb2a6 100644 (file)
--- a/src/i965_avc_bsd.c
+++ b/src/i965_avc_bsd.c
@@ -51,6 +51,7 @@ i965_avc_bsd_init_avc_bsd_surface(VADriverContextP ctx,
 
     if (!avc_bsd_surface) {
         avc_bsd_surface = calloc(sizeof(GenAvcSurface), 1);
+        avc_bsd_surface->frame_store_id = -1;
         assert((obj_surface->size & 0x3f) == 0);
         obj_surface->private_data = avc_bsd_surface;
     }
@@ -795,7 +796,8 @@ i965_avc_bsd_pipeline(VADriverContextP ctx, struct decode_state *decode_state, v
 
     assert(decode_state->pic_param && decode_state->pic_param->buffer);
     pic_param = (VAPictureParameterBufferH264 *)decode_state->pic_param->buffer;
-    intel_update_avc_frame_store_index(ctx, decode_state, pic_param, i965_h264_context->fsid_list);
+    intel_update_avc_frame_store_index(ctx, decode_state, pic_param,
+        i965_h264_context->fsid_list, &i965_h264_context->fs_ctx);
 
     i965_h264_context->enable_avc_ildb = 0;
     i965_h264_context->picture.i_flag = 1;

diff --git a/src/i965_decoder.h b/src/i965_decoder.h
index 01c093f..14d4d0c 100644 (file)
--- a/src/i965_decoder.h
+++ b/src/i965_decoder.h
@@ -39,6 +39,21 @@ struct gen_frame_store {
     VASurfaceID surface_id;
     int         frame_store_id;
     struct      object_surface *obj_surface;
+
+    /* This represents the time when this frame store was last used to
+       hold a reference frame. This is not connected to a presentation
+       timestamp (PTS), and this is not a common decoding time stamp
+       (DTS) either. It serves the purpose of tracking retired
+       reference frame candidates.
+
+       This is only used for H.264 decoding on platforms before Haswell */
+    uint64_t    ref_age;
+};
+
+typedef struct gen_frame_store_context GenFrameStoreContext;
+struct gen_frame_store_context {
+    uint64_t    age;
+    int         prev_poc;
 };
 
 typedef struct gen_buffer GenBuffer;

diff --git a/src/i965_decoder_utils.c b/src/i965_decoder_utils.c
index 7833919..0539e08 100644 (file)
--- a/src/i965_decoder_utils.c
+++ b/src/i965_decoder_utils.c
@@ -22,10 +22,11 @@
  */
 
 #include "sysdeps.h"
-
+#include <limits.h>
 #include <alloca.h>
 
 #include "intel_batchbuffer.h"
+#include "intel_media.h"
 #include "i965_drv_video.h"
 #include "i965_decoder_utils.h"
 #include "i965_defines.h"
@@ -254,6 +255,21 @@ avc_gen_default_iq_matrix(VAIQMatrixBufferH264 *iq_matrix)
     memset(&iq_matrix->ScalingList8x8, 16, sizeof(iq_matrix->ScalingList8x8));
 }
 
+/* Returns the POC of the supplied VA picture */
+static int
+avc_get_picture_poc(const VAPictureH264 *va_pic)
+{
+    int structure, field_poc[2];
+
+    structure = va_pic->flags &
+        (VA_PICTURE_H264_TOP_FIELD | VA_PICTURE_H264_BOTTOM_FIELD);
+    field_poc[0] = structure != VA_PICTURE_H264_BOTTOM_FIELD ?
+        va_pic->TopFieldOrderCnt : INT_MAX;
+    field_poc[1] = structure != VA_PICTURE_H264_TOP_FIELD ?
+        va_pic->BottomFieldOrderCnt : INT_MAX;
+    return MIN(field_poc[0], field_poc[1]);
+}
+
 /* Returns a unique picture ID that represents the supplied VA surface object */
 int
 avc_get_picture_id(struct object_surface *obj_surface)
@@ -471,68 +487,88 @@ gen6_send_avc_ref_idx_state(
     );
 }
 
+/* Comparison function for sorting out the array of free frame store entries */
+static int
+compare_avc_ref_store_func(const void *p1, const void *p2)
+{
+    const GenFrameStore * const fs1 = *((GenFrameStore **)p1);
+    const GenFrameStore * const fs2 = *((GenFrameStore **)p2);
+
+    return fs1->ref_age - fs2->ref_age;
+}
+
 void
 intel_update_avc_frame_store_index(
     VADriverContextP              ctx,
     struct decode_state          *decode_state,
     VAPictureParameterBufferH264 *pic_param,
-    GenFrameStore                 frame_store[MAX_GEN_REFERENCE_FRAMES]
+    GenFrameStore                 frame_store[MAX_GEN_REFERENCE_FRAMES],
+    GenFrameStoreContext         *fs_ctx
 )
 {
     GenFrameStore *free_refs[MAX_GEN_REFERENCE_FRAMES];
-    int i, j, n, num_free_refs;
-
-    /* Remove obsolete entries from the internal DPB */
-    for (i = 0, n = 0; i < MAX_GEN_REFERENCE_FRAMES; i++) {
-        GenFrameStore * const fs = &frame_store[i];
-        if (fs->surface_id == VA_INVALID_ID || !fs->obj_surface) {
-            free_refs[n++] = fs;
+    uint32_t used_refs = 0, add_refs = 0;
+    uint64_t age;
+    int i, n, num_free_refs;
+
+    /* Detect changes of access unit */
+    const int poc = avc_get_picture_poc(&pic_param->CurrPic);
+    if (fs_ctx->age == 0 || fs_ctx->prev_poc != poc)
+        fs_ctx->age++;
+    fs_ctx->prev_poc = poc;
+    age = fs_ctx->age;
+
+    /* Tag entries that are still available in our Frame Store */
+    for (i = 0; i < ARRAY_ELEMS(decode_state->reference_objects); i++) {
+        struct object_surface * const obj_surface =
+            decode_state->reference_objects[i];
+        if (!obj_surface)
             continue;
-        }
 
-        // Find whether the current entry is still a valid reference frame
-        for (j = 0; j < ARRAY_ELEMS(decode_state->reference_objects); j++) {
-            struct object_surface * const obj_surface =
-                decode_state->reference_objects[j];
-            if (obj_surface && obj_surface == fs->obj_surface)
-                break;
+        GenAvcSurface * const avc_surface = obj_surface->private_data;
+        if (avc_surface->frame_store_id >= 0) {
+            GenFrameStore * const fs =
+                &frame_store[avc_surface->frame_store_id];
+            if (fs->surface_id == obj_surface->base.id) {
+                fs->obj_surface = obj_surface;
+                fs->ref_age = age;
+                used_refs |= 1 << fs->frame_store_id;
+                continue;
+            }
         }
+        add_refs |= 1 << i;
+    }
 
-        // ... or remove it
-        if (j == ARRAY_ELEMS(decode_state->reference_objects)) {
-            fs->surface_id = VA_INVALID_ID;
+    /* Build and sort out the list of retired candidates. The resulting
+       list is ordered by increasing age when they were last used */
+    for (i = 0, n = 0; i < MAX_GEN_REFERENCE_FRAMES; i++) {
+        if (!(used_refs & (1 << i))) {
+            GenFrameStore * const fs = &frame_store[i];
             fs->obj_surface = NULL;
-            fs->frame_store_id = -1;
             free_refs[n++] = fs;
         }
     }
     num_free_refs = n;
+    qsort(&free_refs[0], n, sizeof(free_refs[0]), compare_avc_ref_store_func);
 
     /* Append the new reference frames */
     for (i = 0, n = 0; i < ARRAY_ELEMS(decode_state->reference_objects); i++) {
         struct object_surface * const obj_surface =
             decode_state->reference_objects[i];
-        if (!obj_surface)
+        if (!obj_surface || !(add_refs & (1 << i)))
             continue;
 
-        // Find whether the current frame is not already in our frame store
-        for (j = 0; j < MAX_GEN_REFERENCE_FRAMES; j++) {
-            GenFrameStore * const fs = &frame_store[j];
-            if (fs->obj_surface == obj_surface)
-                break;
-        }
-
-        // ... or add it
-        if (j == MAX_GEN_REFERENCE_FRAMES) {
-            if (n < num_free_refs) {
-                GenFrameStore * const fs = free_refs[n++];
-                fs->surface_id = obj_surface->base.id;
-                fs->obj_surface = obj_surface;
-                fs->frame_store_id = fs - frame_store;
-                continue;
-            }
-            WARN_ONCE("No free slot found for DPB reference list!!!\n");
+        GenAvcSurface * const avc_surface = obj_surface->private_data;
+        if (n < num_free_refs) {
+            GenFrameStore * const fs = free_refs[n++];
+            fs->surface_id = obj_surface->base.id;
+            fs->obj_surface = obj_surface;
+            fs->frame_store_id = fs - frame_store;
+            fs->ref_age = age;
+            avc_surface->frame_store_id = fs->frame_store_id;
+            continue;
         }
+        WARN_ONCE("No free slot found for DPB reference list!!!\n");
     }
 }
 

diff --git a/src/i965_decoder_utils.h b/src/i965_decoder_utils.h
index 0ffbd7f..3d39b21 100644 (file)
--- a/src/i965_decoder_utils.h
+++ b/src/i965_decoder_utils.h
@@ -95,10 +95,13 @@ intel_decoder_sanity_check_input(VADriverContextP ctx,
                                  struct decode_state *decode_state);
 
 void
-intel_update_avc_frame_store_index(VADriverContextP ctx,
-                                   struct decode_state *decode_state,
-                                   VAPictureParameterBufferH264 *pic_param,
-                                   GenFrameStore frame_store[MAX_GEN_REFERENCE_FRAMES]);
+intel_update_avc_frame_store_index(
+    VADriverContextP                    ctx,
+    struct decode_state                *decode_state,
+    VAPictureParameterBufferH264       *pic_param,
+    GenFrameStore                       frame_store[MAX_GEN_REFERENCE_FRAMES],
+    GenFrameStoreContext               *fs_ctx
+);
 
 void
 gen75_update_avc_frame_store_index(

diff --git a/src/i965_media_h264.h b/src/i965_media_h264.h
index 490213c..e507e1d 100644 (file)
--- a/src/i965_media_h264.h
+++ b/src/i965_media_h264.h
@@ -61,6 +61,7 @@ struct i965_h264_context
     struct i965_avc_hw_scoreboard_context avc_hw_scoreboard_context;
     struct i965_avc_ildb_context avc_ildb_context;
 
+    GenFrameStoreContext fs_ctx;
     GenFrameStore fsid_list[MAX_GEN_REFERENCE_FRAMES];
 
     struct i965_kernel avc_kernels[NUM_H264_AVC_KERNELS];

diff --git a/src/intel_media.h b/src/intel_media.h
index b30740a..55136d6 100644 (file)
--- a/src/intel_media.h
+++ b/src/intel_media.h
@@ -39,6 +39,7 @@ struct gen_avc_surface
     dri_bo *dmv_top;
     dri_bo *dmv_bottom;
     int dmv_bottom_flag;
+    int frame_store_id; /* only used for H.264 on earlier generations (<HSW) */
 };
 
 extern void gen_free_avc_surface(void **data);