diff mbox series

[FFmpeg-devel,2/2] lavfi/vaapi: add more factors when using VAProcColorStandardExplicit

Message ID 1587870176-1441-2-git-send-email-fei.w.wang@intel.com
State New
Headers show
Series [FFmpeg-devel,1/2] lavfi/vaapi: check avaliable before set color properties
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Fei Wang April 26, 2020, 3:02 a.m. UTC
Use VAProcColorStandardExplicit only if the color properties all
specificed.

Signed-off-by: Fei Wang <fei.w.wang@intel.com>
---
 libavfilter/vaapi_vpp.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Mark Thompson April 26, 2020, 6:16 p.m. UTC | #1
On 26/04/2020 04:02, Fei Wang wrote:
> Use VAProcColorStandardExplicit only if the color properties all
> specificed.
> 
> Signed-off-by: Fei Wang <fei.w.wang@intel.com>
> ---
>  libavfilter/vaapi_vpp.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/libavfilter/vaapi_vpp.c b/libavfilter/vaapi_vpp.c
> index 6ffc09d..e1d3373 100644
> --- a/libavfilter/vaapi_vpp.c
> +++ b/libavfilter/vaapi_vpp.c
> @@ -278,10 +278,14 @@ static void vaapi_vpp_fill_colour_standard(VAAPIColourProperties *props,
>      // use them and avoid doing any mapping.  (The driver may not support
>      // some particular code point, but it still has enough information to
>      // make a better fallback choice than we do in that case.)
> -    for (i = 0; i < nb_vacs; i++) {
> -        if (vacs[i] == VAProcColorStandardExplicit) {
> -            props->va_color_standard = VAProcColorStandardExplicit;
> -            return;
> +    if ((props->colorspace != AVCOL_SPC_UNSPECIFIED) &&
> +        (props->color_trc != AVCOL_TRC_UNSPECIFIED) &&
> +        (props->color_primaries != AVCOL_PRI_UNSPECIFIED)) {
> +        for (i = 0; i < nb_vacs; i++) {
> +            if (vacs[i] == VAProcColorStandardExplicit) {
> +                props->va_color_standard = VAProcColorStandardExplicit;
> +                return;
> +            }
>          }
>      }
>  #endif

This seems like it's throwing away information in a way you don't want.  For example, suppose the input was decoded from VP9: in that case you only know the matrix coefficients (props->colorspace) and the other values will be unspecified, but that's still better to know than nothing at all.

Alternatively: if you think this should be handled at a layer above VAAPI, maybe it should say that in the libva documentation?  Then here we would want to guess the other values and fill them in.

- Mark
diff mbox series

Patch

diff --git a/libavfilter/vaapi_vpp.c b/libavfilter/vaapi_vpp.c
index 6ffc09d..e1d3373 100644
--- a/libavfilter/vaapi_vpp.c
+++ b/libavfilter/vaapi_vpp.c
@@ -278,10 +278,14 @@  static void vaapi_vpp_fill_colour_standard(VAAPIColourProperties *props,
     // use them and avoid doing any mapping.  (The driver may not support
     // some particular code point, but it still has enough information to
     // make a better fallback choice than we do in that case.)
-    for (i = 0; i < nb_vacs; i++) {
-        if (vacs[i] == VAProcColorStandardExplicit) {
-            props->va_color_standard = VAProcColorStandardExplicit;
-            return;
+    if ((props->colorspace != AVCOL_SPC_UNSPECIFIED) &&
+        (props->color_trc != AVCOL_TRC_UNSPECIFIED) &&
+        (props->color_primaries != AVCOL_PRI_UNSPECIFIED)) {
+        for (i = 0; i < nb_vacs; i++) {
+            if (vacs[i] == VAProcColorStandardExplicit) {
+                props->va_color_standard = VAProcColorStandardExplicit;
+                return;
+            }
         }
     }
 #endif