Add informational messages for developers on failed H.264 checks

Includes a warning if the level appears to be invalid, but continues anyway
in this case (it could be a correct future value or an unknown extension).

Signed-off-by: Mark Thompson <sw@jkqxz.net>

4 files changed, 40 insertions, 17 deletions

diff --git a/src/i965_avc_encoder_common.c b/src/i965_avc_encoder_common.c
index c3f69e3c..464ed500 100644
--- a/src/i965_avc_encoder_common.c
+++ b/src/i965_avc_encoder_common.c
@@ -71,6 +71,12 @@ get_level_limits(int level_idc)
 }
 
 int
+i965_avc_level_is_valid(int level_idc)
+{
+    return get_level_limits(level_idc)->level_idc == level_idc;
+}
+
+int
 i965_avc_get_max_mbps(int level_idc)
 {
     return get_level_limits(level_idc)->max_mbps;
diff --git a/src/i965_avc_encoder_common.h b/src/i965_avc_encoder_common.h
index c42a4a55..d69686fd 100644
--- a/src/i965_avc_encoder_common.h
+++ b/src/i965_avc_encoder_common.h
@@ -322,6 +322,7 @@ struct avc_enc_state {
 
 };
 
+extern int i965_avc_level_is_valid(int level_idc);
 extern int i965_avc_get_max_mbps(int level_idc);
 extern int i965_avc_calculate_initial_qp(struct avc_param * param);
 extern unsigned int i965_avc_get_profile_level_max_frame(struct avc_param * param, int level_idc);
diff --git a/src/i965_encoder.c b/src/i965_encoder.c
index 4c2fc037..7e0668af 100644
--- a/src/i965_encoder.c
+++ b/src/i965_encoder.c
@@ -42,6 +42,7 @@
 
 #include "i965_post_processing.h"
 #include "i965_encoder_api.h"
+#include "i965_avc_encoder_common.h"
 
 static struct intel_fraction
 reduce_fraction(struct intel_fraction f)
@@ -852,6 +853,8 @@ intel_encoder_check_misc_parameter(VADriverContextP ctx,
                 break;
             }
         } else if (encoder_context->quality_level > encoder_context->quality_range) {
+            i965_log_info(ctx, "VAEncMiscParameterBufferQualityLevel.quality_level (%d) out of range (max %d).\n",
+                          encoder_context->quality_level, encoder_context->quality_range);
             ret = VA_STATUS_ERROR_INVALID_PARAMETER;
             goto out;
         }
@@ -880,26 +883,41 @@ intel_encoder_check_avc_parameter(VADriverContextP ctx,
     VAEncSequenceParameterBufferH264 *seq_param = (VAEncSequenceParameterBufferH264 *)encode_state->seq_param_ext->buffer;
     int i;
 
-    assert(!(pic_param->CurrPic.flags & VA_PICTURE_H264_INVALID));
+    if (seq_param->level_idc != encoder_context->codec_level &&
+        !i965_avc_level_is_valid(seq_param->level_idc)) {
+        i965_log_info(ctx, "VAEncSequenceParameterBufferH264.level_idc (%d) does not appear to be valid.\n",
+                      seq_param->level_idc);
+        encoder_context->codec_level = seq_param->level_idc;
+        // Only print this the first time we see it, and continue anyway - this could be a correct future
+        // value or an unknown extension.
+    }
 
-    if (pic_param->CurrPic.flags & VA_PICTURE_H264_INVALID)
+    if (pic_param->CurrPic.flags & VA_PICTURE_H264_INVALID) {
+        i965_log_info(ctx, "VAEncPictureParameterBufferH264.CurrPic.flags (%#x) is invalid.\n",
+                      pic_param->CurrPic.flags);
         goto error;
+    }
 
     obj_surface = SURFACE(pic_param->CurrPic.picture_id);
-    assert(obj_surface); /* It is possible the store buffer isn't allocated yet */
-
-    if (!obj_surface)
+    if (!obj_surface) {
+        i965_log_info(ctx, "VAEncPictureParameterBufferH264.CurrPic.picture_id (%#x) is not a valid surface.\n",
+                      pic_param->CurrPic.picture_id);
         goto error;
+    }
 
     encode_state->reconstructed_object = obj_surface;
     obj_buffer = BUFFER(pic_param->coded_buf);
-    assert(obj_buffer && obj_buffer->buffer_store && obj_buffer->buffer_store->bo);
-
-    if (!obj_buffer || !obj_buffer->buffer_store || !obj_buffer->buffer_store->bo)
+    if (!obj_buffer || !obj_buffer->buffer_store || !obj_buffer->buffer_store->bo) {
+        i965_log_info(ctx, "VAEncPictureParameterBufferH264.coded_buf (%#x) is not a valid buffer.\n",
+                      pic_param->coded_buf);
         goto error;
+    }
 
-    if (encode_state->num_slice_params_ext > encoder_context->max_slice_or_seg_num)
+    if (encode_state->num_slice_params_ext > encoder_context->max_slice_or_seg_num) {
+        i965_log_info(ctx, "Too many slices in picture submission: %d, max supported is %d.\n",
+                      encode_state->num_slice_params_ext, encoder_context->max_slice_or_seg_num);
         goto error;
+    }
 
     encode_state->coded_buf_object = obj_buffer;
 
@@ -909,15 +927,12 @@ intel_encoder_check_avc_parameter(VADriverContextP ctx,
             break;
         else {
             obj_surface = SURFACE(pic_param->ReferenceFrames[i].picture_id);
-            assert(obj_surface);
-
-            if (!obj_surface)
+            if (!obj_surface || !obj_surface->bo) {
+                i965_log_info(ctx, "VAEncPictureParameterBufferH264.ReferenceFrames[%d].picture_id (%#x)"
+                              " is not a valid surface.\n", i, pic_param->ReferenceFrames[i].picture_id);
                 goto error;
-
-            if (obj_surface->bo)
-                encode_state->reference_objects[i] = obj_surface;
-            else
-                encode_state->reference_objects[i] = NULL; /* FIXME: Warning or Error ??? */
+            }
+            encode_state->reference_objects[i] = obj_surface;
         }
     }
 
diff --git a/src/i965_encoder.h b/src/i965_encoder.h
index 23a0b10a..0a06cd5e 100644
--- a/src/i965_encoder.h
+++ b/src/i965_encoder.h
@@ -71,6 +71,7 @@ struct intel_encoder_context {
     unsigned int frame_width_in_pixel;
     unsigned int frame_height_in_pixel;
     unsigned int max_slice_or_seg_num;
+    unsigned int codec_level;
 
     struct {
         unsigned int num_layers;