diff mbox

[FFmpeg-devel] avcodec/vaapi_encode: modify the initialization postion of profile_string

Message ID 20181105093923.32626-1-linjie.fu@intel.com
State New
Headers show

Commit Message

Fu, Linjie Nov. 5, 2018, 9:39 a.m. UTC
Currently, profile_string was initialized in the "for" loop. If
it didn't enter this loop or accidently break, profile_string may be
uninitialized.

Modify to initialize the profile_string after the loop to avoid
using the uninitialized value when calling av_log.

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 libavcodec/vaapi_encode.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Zhong Li Nov. 5, 2018, 12:35 p.m. UTC | #1
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Linjie Fu

> Sent: Monday, November 5, 2018 5:39 PM

> To: ffmpeg-devel@ffmpeg.org

> Cc: Fu, Linjie <linjie.fu@intel.com>

> Subject: [FFmpeg-devel] [PATCH] avcodec/vaapi_encode: modify the

> initialization postion of profile_string

> 

> Currently, profile_string was initialized in the "for" loop. If it didn't enter this

> loop or accidently break, profile_string may be uninitialized.

> 

> Modify to initialize the profile_string after the loop to avoid using the

> uninitialized value when calling av_log.

> 

> Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> ---

>  libavcodec/vaapi_encode.c | 12 ++++++------

>  1 file changed, 6 insertions(+), 6 deletions(-)

> 

> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index

> 2c34cdce2c..b43b52f0e5 100644

> --- a/libavcodec/vaapi_encode.c

> +++ b/libavcodec/vaapi_encode.c

> @@ -1066,6 +1066,7 @@ static av_cold int

> vaapi_encode_profile_entrypoint(AVCodecContext *avctx)

>      }

> 

>      av_assert0(ctx->codec->profiles);

> +


Please don't introduce redundant line.

>      for (i = 0; (ctx->codec->profiles[i].av_profile !=

>                   FF_PROFILE_UNKNOWN); i++) {

>          profile = &ctx->codec->profiles[i]; @@ -1080,12 +1081,6 @@

> static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)

>              avctx->profile != FF_PROFILE_UNKNOWN)

>              continue;

> 

> -#if VA_CHECK_VERSION(1, 0, 0)

> -        profile_string = vaProfileStr(profile->va_profile);

> -#else

> -        profile_string = "(no profile names)";

> -#endif

> -

>          for (j = 0; j < n; j++) {

>              if (va_profiles[j] == profile->va_profile)

>                  break;

> @@ -1107,6 +1102,11 @@ static av_cold int

> vaapi_encode_profile_entrypoint(AVCodecContext *avctx)

> 

>      avctx->profile  = profile->av_profile;

>      ctx->va_profile = profile->va_profile;

> +#if VA_CHECK_VERSION(1, 0, 0)

> +    profile_string = vaProfileStr(profile->va_profile);

> +#else

> +    profile_string = "(no profile names)"; #endif

>      av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI profile %s (%d).\n",

>             profile_string, ctx->va_profile);

> 

> --

> 2.17.1
Mark Thompson Nov. 5, 2018, 5:58 p.m. UTC | #2
On 05/11/18 09:39, Linjie Fu wrote:
> Currently, profile_string was initialized in the "for" loop. If
> it didn't enter this loop or accidently break, profile_string may be
> uninitialized.
> 
> Modify to initialize the profile_string after the loop to avoid
> using the uninitialized value when calling av_log.
> 
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  libavcodec/vaapi_encode.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 2c34cdce2c..b43b52f0e5 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -1066,6 +1066,7 @@ static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
>      }
>  
>      av_assert0(ctx->codec->profiles);
> +
>      for (i = 0; (ctx->codec->profiles[i].av_profile !=
>                   FF_PROFILE_UNKNOWN); i++) {
>          profile = &ctx->codec->profiles[i];
> @@ -1080,12 +1081,6 @@ static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
>              avctx->profile != FF_PROFILE_UNKNOWN)
>              continue;
>  
> -#if VA_CHECK_VERSION(1, 0, 0)
> -        profile_string = vaProfileStr(profile->va_profile);
> -#else
> -        profile_string = "(no profile names)";
> -#endif
> -
>          for (j = 0; j < n; j++) {
>              if (va_profiles[j] == profile->va_profile)
>                  break;
> @@ -1107,6 +1102,11 @@ static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
>  
>      avctx->profile  = profile->av_profile;
>      ctx->va_profile = profile->va_profile;
> +#if VA_CHECK_VERSION(1, 0, 0)
> +    profile_string = vaProfileStr(profile->va_profile);
> +#else
> +    profile_string = "(no profile names)";
> +#endif
>      av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI profile %s (%d).\n",
>             profile_string, ctx->va_profile);
>  
> 

I'm not seeing where this goes wrong?  ctx->profile isn't set if you exit the loop early.

The profile string name should be in the message about unsupported profiles, though.  I'll send a patch for that.

- Mark
diff mbox

Patch

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 2c34cdce2c..b43b52f0e5 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1066,6 +1066,7 @@  static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
     }
 
     av_assert0(ctx->codec->profiles);
+
     for (i = 0; (ctx->codec->profiles[i].av_profile !=
                  FF_PROFILE_UNKNOWN); i++) {
         profile = &ctx->codec->profiles[i];
@@ -1080,12 +1081,6 @@  static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
             avctx->profile != FF_PROFILE_UNKNOWN)
             continue;
 
-#if VA_CHECK_VERSION(1, 0, 0)
-        profile_string = vaProfileStr(profile->va_profile);
-#else
-        profile_string = "(no profile names)";
-#endif
-
         for (j = 0; j < n; j++) {
             if (va_profiles[j] == profile->va_profile)
                 break;
@@ -1107,6 +1102,11 @@  static av_cold int vaapi_encode_profile_entrypoint(AVCodecContext *avctx)
 
     avctx->profile  = profile->av_profile;
     ctx->va_profile = profile->va_profile;
+#if VA_CHECK_VERSION(1, 0, 0)
+    profile_string = vaProfileStr(profile->va_profile);
+#else
+    profile_string = "(no profile names)";
+#endif
     av_log(avctx, AV_LOG_VERBOSE, "Using VAAPI profile %s (%d).\n",
            profile_string, ctx->va_profile);