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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to