On 09/07/2016 11:01 AM, Xiang, Haihao wrote:

i965_encoder.c is a general file, it would be better not to include
more HW/implementation related code in this file.

Actually it is more clear if you look into the new
gen9_vme_context_init() and gen9_mfc_context_init(). Previous it
selects different path both in gen9_enc_hw_context_init(),
gen9_enc_hw_context_init(), gen9_mfc_context_init() per codec.

I see it.

In fact there is no much difference.

Of course it is still OK to me that the callback function is selected in gen9_vme_context_init/gen9_mfc_context_init.

Thanks
   Yakui

Thanks
Haihao




On 09/06/2016 11:39 PM, Xiang, Haihao wrote:
It keeps i965_encoder.c simple


Thanks for the patch.
But I don't think that this patch is necessary. The code looks more
clear if it can select the different initialization callback function
earlier based on the corresponding profile/entrypoint . At the same
time
it can also avoid that the gen9_vme.c/gen9_mfc.c calls the other API
implementation.

Signed-off-by: Xiang, Haihao<haihao.xi...@intel.com>
---
   src/gen6_mfc.h     |  6 ++++++
   src/gen6_vme.h     |  2 +-
   src/gen9_mfc.c     | 25 +++++++++++++++++++------
   src/gen9_vme.c     | 15 ++++++++++++++-
   src/i965_encoder.c | 37 ++++++-------------------------------
   5 files changed, 46 insertions(+), 39 deletions(-)

diff --git a/src/gen6_mfc.h b/src/gen6_mfc.h
index 4561d43..702596b 100644
--- a/src/gen6_mfc.h
+++ b/src/gen6_mfc.h
@@ -346,6 +346,12 @@ VAStatus gen6_mfc_pipeline(VADriverContextP
ctx,
   void gen6_mfc_context_destroy(void *context);

   extern
+Bool gen6_mfc_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context);
+
+extern
+Bool gen7_mfc_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context);
+
+extern
   Bool gen75_mfc_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context);


diff --git a/src/gen6_vme.h b/src/gen6_vme.h
index e8f4742..f94085f 100644
--- a/src/gen6_vme.h
+++ b/src/gen6_vme.h
@@ -116,7 +116,7 @@ struct gen6_vme_context
   #define      MPEG2_LEVEL_MAIN        0x08
   #define      MPEG2_LEVEL_HIGH        0x04

-
+Bool gen6_vme_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context);
   Bool gen75_vme_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context);

   extern void intel_vme_update_mbmv_cost(VADriverContextP ctx,
diff --git a/src/gen9_mfc.c b/src/gen9_mfc.c
index b3d6e78..ce038b1 100644
--- a/src/gen9_mfc.c
+++ b/src/gen9_mfc.c
@@ -39,18 +39,31 @@
   #include "i965_drv_video.h"
   #include "i965_encoder.h"
   #include "gen6_mfc.h"
+#include "gen9_mfc.h"
+#include "gen9_vdenc.h"
+#include "gen9_vp9_encapi.h"

   Bool gen9_mfc_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context)
   {
-    if ((encoder_context->codec == CODEC_H264) ||
-        (encoder_context->codec == CODEC_H264_MVC)) {
+    switch (encoder_context->codec) {
+    case CODEC_VP8:
+    case CODEC_MPEG2:
+    case CODEC_JPEG:
+        return gen8_mfc_context_init(ctx, encoder_context);
+
+    case CODEC_H264:
+    case CODEC_H264_MVC:
+        if (encoder_context->low_power_mode)
+            return gen9_vdenc_context_init(ctx, encoder_context);
+        else
               return gen8_mfc_context_init(ctx, encoder_context);
-    }

+    case CODEC_HEVC:
+        return gen9_hcpe_context_init(ctx, encoder_context);

-    if ((encoder_context->codec == CODEC_VP8) ||
-        (encoder_context->codec == CODEC_MPEG2))
-        return gen8_mfc_context_init(ctx, encoder_context);
+    case CODEC_VP9:
+        return gen9_vp9_pak_context_init(ctx, encoder_context);
+    }

       /* Other profile/entrypoint pairs never get here, see
gen9_enc_hw_context_init() */
       assert(0);
diff --git a/src/gen9_vme.c b/src/gen9_vme.c
index 1625c2b..4f19409 100644
--- a/src/gen9_vme.c
+++ b/src/gen9_vme.c
@@ -40,6 +40,7 @@
   #include "i965_encoder.h"
   #include "gen6_vme.h"
   #include "gen6_mfc.h"
+#include "gen9_vp9_encapi.h"

   #ifdef SURFACE_STATE_PADDED_SIZE
   #undef SURFACE_STATE_PADDED_SIZE
@@ -1817,10 +1818,22 @@ gen9_vme_context_destroy(void *context)

   Bool gen9_vme_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context)
   {
-    struct gen6_vme_context *vme_context = calloc(1, sizeof(struct
gen6_vme_context));
+    struct gen6_vme_context *vme_context;
       struct i965_kernel *vme_kernel_list = NULL;
       int i965_kernel_num;

+    if (encoder_context->low_power_mode || encoder_context->codec
== CODEC_JPEG) {
+        encoder_context->vme_context = NULL;
+        encoder_context->vme_pipeline = NULL;
+        encoder_context->vme_context_destroy = NULL;
+
+        return True;
+    } else if (encoder_context->codec == CODEC_VP9) {
+        return gen9_vp9_vme_context_init(ctx, encoder_context);
+    }
+
+    vme_context = calloc(1, sizeof(struct gen6_vme_context));
+
       switch (encoder_context->codec) {
       case CODEC_H264:
       case CODEC_H264_MVC:
diff --git a/src/i965_encoder.c b/src/i965_encoder.c
index 47368fb..be01e83 100644
--- a/src/i965_encoder.c
+++ b/src/i965_encoder.c
@@ -39,15 +39,6 @@
   #include "i965_encoder.h"
   #include "gen6_vme.h"
   #include "gen6_mfc.h"
-#include "gen9_mfc.h"
-#include "gen9_vdenc.h"
-
-#include "gen9_vp9_encapi.h"
-
-extern Bool gen6_mfc_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context);
-extern Bool gen6_vme_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context);
-extern Bool gen7_mfc_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context);
-extern Bool gen9_hcpe_context_init(VADriverContextP ctx, struct
intel_encoder_context *encoder_context);

   static VAStatus
   clear_border(struct object_surface *obj_surface)
@@ -836,6 +827,9 @@ intel_enc_hw_context_init(VADriverContextP ctx,
       encoder_context->quality_level = ENCODER_DEFAULT_QUALITY;
       encoder_context->quality_range = 1;

+    if (obj_config->entrypoint == VAEntrypointEncSliceLP)
+        encoder_context->low_power_mode = 1;
+
       switch (obj_config->profile) {
       case VAProfileMPEG2Simple:
       case VAProfileMPEG2Main:
@@ -898,14 +892,8 @@ intel_enc_hw_context_init(VADriverContextP
ctx,

       if (vme_context_init) {
           vme_context_init(ctx, encoder_context);
-
-        if (obj_config->profile != VAProfileJPEGBaseline) {
-            assert(encoder_context->vme_context);
-            assert(encoder_context->vme_context_destroy);
-            assert(encoder_context->vme_pipeline);
-        }
-    } else {
-        encoder_context->low_power_mode = 1;
+        assert(!encoder_context->vme_context ||
+               (encoder_context-
vme_context_destroy&&   encoder_context->vme_pipeline));
       }

       mfc_context_init(ctx, encoder_context);
@@ -944,18 +932,5 @@ gen8_enc_hw_context_init(VADriverContextP ctx,
struct object_config *obj_config)
   struct hw_context *
   gen9_enc_hw_context_init(VADriverContextP ctx, struct
object_config *obj_config)
   {
-    if (obj_config->entrypoint == VAEntrypointEncSliceLP) {
-        return intel_enc_hw_context_init(ctx, obj_config, NULL,
gen9_vdenc_context_init);
-    } else {
-        if (obj_config->profile == VAProfileHEVCMain) {
-            return intel_enc_hw_context_init(ctx, obj_config,
gen9_vme_context_init, gen9_hcpe_context_init);
-        } else if (obj_config->profile == VAProfileJPEGBaseline)
-            return intel_enc_hw_context_init(ctx, obj_config,
gen8_vme_context_init, gen8_mfc_context_init);
-        else if (obj_config->profile == VAProfileVP9Profile0)
-            return intel_enc_hw_context_init(ctx, obj_config,
-                                             gen9_vp9_vme_context_
init,
-                                             gen9_vp9_pak_context_
init);
-        else
-            return intel_enc_hw_context_init(ctx, obj_config,
gen9_vme_context_init, gen9_mfc_context_init);
-    }
+    return intel_enc_hw_context_init(ctx, obj_config,
gen9_vme_context_init, gen9_mfc_context_init);
   }

_______________________________________________
Libva mailing list
Libva@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libva

Reply via email to