diff mbox

[FFmpeg-devel] HW accelerator: Indicate when HW acceleration is in use

Message ID 1509734133-15368-1-git-send-email-michele.lim@intel.com
State New
Headers show

Commit Message

Michele Lim Nov. 3, 2017, 6:35 p.m. UTC
Having clear indication of when a hardware accelerator is in
operation prevents false assumptions, for e.g., in situations when
the command line argument inadvertently omits options for enabling
it, resulting to the framework silently switching to the SW path.

Signed-off-by: Michele Lim <michele.lim@intel.com>
---
 fftools/ffmpeg_hw.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Michael Niedermayer Nov. 4, 2017, 3:16 a.m. UTC | #1
On Fri, Nov 03, 2017 at 11:35:33AM -0700, Michele Lim wrote:
> Having clear indication of when a hardware accelerator is in
> operation prevents false assumptions, for e.g., in situations when
> the command line argument inadvertently omits options for enabling
> it, resulting to the framework silently switching to the SW path.
> 
> Signed-off-by: Michele Lim <michele.lim@intel.com>
> ---
>  fftools/ffmpeg_hw.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c
> index a4d1cad..f071746 100644
> --- a/fftools/ffmpeg_hw.c
> +++ b/fftools/ffmpeg_hw.c
> @@ -306,6 +306,8 @@ int hw_device_setup_for_decode(InputStream *ist)
>      if (!ist->dec_ctx->hw_device_ctx)
>          return AVERROR(ENOMEM);
>  
> +    /* Indicate HW accelerator has been prepared for decode */
> +    av_log(ist->dec_ctx, AV_LOG_INFO, "HW accelerator prepared for decode: %s\n", av_hwdevice_get_type_name(type));
>      return 0;
>  }
>  

> @@ -331,6 +333,9 @@ int hw_device_setup_for_encode(OutputStream *ost)
>          // No device required.
>          return 0;
>      }
> +
> +    /* Indicate HW accelerator has been prepared for encode */
> +    av_log(ost->enc_ctx, AV_LOG_INFO, "HW accelerator prepared for encode: %s\n", av_hwdevice_get_type_name(type));
>  }

i dont know if this extra info is desireable or not but
this code is unreachable

[...]
Mark Thompson Nov. 4, 2017, 7:08 p.m. UTC | #2
On 03/11/17 18:35, Michele Lim wrote:
> Having clear indication of when a hardware accelerator is in
> operation prevents false assumptions, for e.g., in situations when
> the command line argument inadvertently omits options for enabling
> it, resulting to the framework silently switching to the SW path.
> 
> Signed-off-by: Michele Lim <michele.lim@intel.com>
> ---
>  fftools/ffmpeg_hw.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c
> index a4d1cad..f071746 100644
> --- a/fftools/ffmpeg_hw.c
> +++ b/fftools/ffmpeg_hw.c
> @@ -306,6 +306,8 @@ int hw_device_setup_for_decode(InputStream *ist)
>      if (!ist->dec_ctx->hw_device_ctx)
>          return AVERROR(ENOMEM);
>  
> +    /* Indicate HW accelerator has been prepared for decode */
> +    av_log(ist->dec_ctx, AV_LOG_INFO, "HW accelerator prepared for decode: %s\n", av_hwdevice_get_type_name(type));
>      return 0;
>  }
>  
> @@ -331,6 +333,9 @@ int hw_device_setup_for_encode(OutputStream *ost)
>          // No device required.
>          return 0;
>      }
> +
> +    /* Indicate HW accelerator has been prepared for encode */
> +    av_log(ost->enc_ctx, AV_LOG_INFO, "HW accelerator prepared for encode: %s\n", av_hwdevice_get_type_name(type));
>  }
>  
>  static int hwaccel_retrieve_data(AVCodecContext *avctx, AVFrame *input)
> 

I don't think this does what you want.  It is only a preparation step which sets hardware devices which might then be used by a codec, and tells you little about whether it is actually used in practice.  Consider that your message will appear when setting -hwaccel_device and decoding with a random codec with no hardware support, and also when a codec does support some hardware but not the actual stream being decoded (e.g. H.264 4:2:2).

If you want a message like this (which I admit ambivalence to, but if other people think it's useful then sure), I think it has to be logged later when we actually know whether the hardware is usable - either inside lavc or in ffmpeg.c after a frame is returned.

- Mark
Michele Lim Nov. 5, 2017, 10:33 p.m. UTC | #3
Mark, 

Thanks for your response.  I agree with you and actually had an implementation similar to what you've described for the decoder but abandoned it; I haven't found an elegant way without making too many changes to indicate it much later when hardware is actually set up for execution.  This is being handled in a loop for all possible pixel formats supported by the display surface. (for my display there are 9, so the same message is logged 9 times).

A more elegant solution will require additional code which is against FFmpeg guideline.  I think this benefits everyone wanting assurance as to whether the coding or decoding is done using the hardware or software path.

Meanwhile, I can submit with the not-so-elegant solution so that you can see what I mean - this is just for the decoder.

I will look into adding more code for a more elegant solution, if that's acceptable.  Please let me know which way is best.

Thanks,
Michele

-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Mark Thompson

Sent: Saturday, November 4, 2017 12:09 PM
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] HW accelerator: Indicate when HW acceleration is in use

On 03/11/17 18:35, Michele Lim wrote:
> Having clear indication of when a hardware accelerator is in operation 

> prevents false assumptions, for e.g., in situations when the command 

> line argument inadvertently omits options for enabling it, resulting 

> to the framework silently switching to the SW path.

> 

> Signed-off-by: Michele Lim <michele.lim@intel.com>

> ---

>  fftools/ffmpeg_hw.c | 5 +++++

>  1 file changed, 5 insertions(+)

> 

> diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c index 

> a4d1cad..f071746 100644

> --- a/fftools/ffmpeg_hw.c

> +++ b/fftools/ffmpeg_hw.c

> @@ -306,6 +306,8 @@ int hw_device_setup_for_decode(InputStream *ist)

>      if (!ist->dec_ctx->hw_device_ctx)

>          return AVERROR(ENOMEM);

>  

> +    /* Indicate HW accelerator has been prepared for decode */

> +    av_log(ist->dec_ctx, AV_LOG_INFO, "HW accelerator prepared for 

> + decode: %s\n", av_hwdevice_get_type_name(type));

>      return 0;

>  }

>  

> @@ -331,6 +333,9 @@ int hw_device_setup_for_encode(OutputStream *ost)

>          // No device required.

>          return 0;

>      }

> +

> +    /* Indicate HW accelerator has been prepared for encode */

> +    av_log(ost->enc_ctx, AV_LOG_INFO, "HW accelerator prepared for 

> + encode: %s\n", av_hwdevice_get_type_name(type));

>  }

>  

>  static int hwaccel_retrieve_data(AVCodecContext *avctx, AVFrame 

> *input)

> 


I don't think this does what you want.  It is only a preparation step which sets hardware devices which might then be used by a codec, and tells you little about whether it is actually used in practice.  Consider that your message will appear when setting -hwaccel_device and decoding with a random codec with no hardware support, and also when a codec does support some hardware but not the actual stream being decoded (e.g. H.264 4:2:2).

If you want a message like this (which I admit ambivalence to, but if other people think it's useful then sure), I think it has to be logged later when we actually know whether the hardware is usable - either inside lavc or in ffmpeg.c after a frame is returned.

- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c
index a4d1cad..f071746 100644
--- a/fftools/ffmpeg_hw.c
+++ b/fftools/ffmpeg_hw.c
@@ -306,6 +306,8 @@  int hw_device_setup_for_decode(InputStream *ist)
     if (!ist->dec_ctx->hw_device_ctx)
         return AVERROR(ENOMEM);
 
+    /* Indicate HW accelerator has been prepared for decode */
+    av_log(ist->dec_ctx, AV_LOG_INFO, "HW accelerator prepared for decode: %s\n", av_hwdevice_get_type_name(type));
     return 0;
 }
 
@@ -331,6 +333,9 @@  int hw_device_setup_for_encode(OutputStream *ost)
         // No device required.
         return 0;
     }
+
+    /* Indicate HW accelerator has been prepared for encode */
+    av_log(ost->enc_ctx, AV_LOG_INFO, "HW accelerator prepared for encode: %s\n", av_hwdevice_get_type_name(type));
 }
 
 static int hwaccel_retrieve_data(AVCodecContext *avctx, AVFrame *input)