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
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".