[FFmpeg-devel] [PATCH v4 5/7] lavfi/vaapi: Improve support for colour properties

Michael Niedermayer michael at niedermayer.cc
Wed Apr 10 21:06:21 EEST 2019


On Tue, Apr 09, 2019 at 11:07:28PM +0100, Mark Thompson wrote:
> Attempts to pick the set of supported colour properties best matching the
> input.  Output is then set with the same values, except for the colour
> matrix which may change when converting between RGB and YUV.
> ---
> On 04/03/2019 11:36, Song, Ruiling wrote:
> > Why using these magic number instead of meaningful enum value?
> 
> See <https://github.com/intel/libva/blob/master/va/va_vpp.h#L304-L402>.  These values are standardised in H.273 (or ISO/IEC 23001-8:2016), which are then used directly in both FFmpeg and various codecs like MPEG-2, H.264, H.265 and AV1.  On balance it seemed clearer to keep the numeric values rather than to turn them into symbolic names which would be much harder to read.
> 
> > And two entries added for VAProcColorStandardBT601, seems that only the first will be returned?
> 
> It was meant to check both cases, now fixed.
> 
> >> +    // Give scores to the possible options and choose the lowest one.
> >> +    // An exact match will score zero and therefore always be chosen, as
> >> +    // will a partial match where all unmatched elements are explicitly
> >> +    // unspecified.  (If all elements are unspecified this will use the
> >> +    // first available value.)  If no options match at all then just
> >> +    // pass "none" to the driver and let it make its own choice.
> > Here (a*4+b*2+c)  is chosen as the score function, I am not sure whether (a + b + c) is just ok?
> 
> I made the weights different to try to avoid confusing draws where it might match to a different colourspace on one run and then a different transfer characteristic on another.
> 
> The choice of matrix > transfer > primaries is mostly arbitrary coming from vague intuition about how extreme the bad cases are (wrong matrix can be wildly incorrect (YCoCg interpreted as YUV, say), wrong transfer can mess up the intensity in a confusing way but should mostly resemble the intended image, wrong primaries will shift colours and look weird but stay be mostly coherent).  I'm happy to change that if you have a different opinion!
> 
> >> +    best_score = -1;
> >> +    worst_score = 4 * (props->colorspace != AVCOL_SPC_UNSPECIFIED) +
> >> +                  2 * (props->color_trc != AVCOL_TRC_UNSPECIFIED) +
> >> +                      (props->color_primaries != AVCOL_PRI_UNSPECIFIED);
> > Seems that the outer loop here is just used to re-iterate through nb_vacs to find the best match again?
> > Can we remove the outer-loop-over-k like below?
> > 
> > best_va_standard = VAProcColorStandardNone;
> > for (i = 0; i < nb_vacs; i++) {
> > 	...
> > 	...
> >                // Only include things which matched something.
> >                 if ((worst_score == 0 || score < worst_score) &&
> >                     (best_score == -1 || score < best_score)) {
> >                     best_score = score;
> > 	    best_va_standard = t->va_color_standard;
> > 	}
> > }
> > props->va_color_standard = best_va_standard;
> 
> Yes, that approach would be nicer.  Done.
> 
> Thanks,
> 
> - Mark
> 
> 
>  libavfilter/vaapi_vpp.c            | 285 +++++++++++++++++++++++++++--
>  libavfilter/vaapi_vpp.h            |   2 -
>  libavfilter/vf_deinterlace_vaapi.c |   8 +-
>  libavfilter/vf_misc_vaapi.c        |   7 +-
>  libavfilter/vf_procamp_vaapi.c     |   7 +-
>  libavfilter/vf_scale_vaapi.c       |   8 +-
>  libavfilter/vf_transpose_vaapi.c   |   7 +-
>  7 files changed, 292 insertions(+), 32 deletions(-)
> 
> diff --git a/libavfilter/vaapi_vpp.c b/libavfilter/vaapi_vpp.c
> index 647ddc0811..5091d2508a 100644
> --- a/libavfilter/vaapi_vpp.c
> +++ b/libavfilter/vaapi_vpp.c

breaks build 
CC	libavfilter/vaapi_vpp.o
libavfilter/vaapi_vpp.c: In function ‘vaapi_vpp_colour_properties’:
libavfilter/vaapi_vpp.c:466:43: error: ‘VAProcColorStandardExplicit’ undeclared (first use in this function)
     if (output_props.va_color_standard != VAProcColorStandardExplicit) {
                                           ^
libavfilter/vaapi_vpp.c:466:43: note: each undeclared identifier is reported only once for each function it appears in
make: *** [libavfilter/vaapi_vpp.o] Error 1
make: Target `all' not remade because of errors.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190410/12903be6/attachment.sig>


More information about the ffmpeg-devel mailing list