> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of > Moritz Barsnick > Sent: Tuesday, December 25, 2018 6:06 PM > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH V1] avfilter: Add tonemap vaapi filter > > On Tue, Dec 25, 2018 at 15:16:17 +0800, Zachary Zhou wrote: > > It supports ICL platform. > > H2H (HDR to HDR): P010 -> RGB10 > > H2S (HDR to SDR): P010 -> RGB8 > > > > libva commit for HDR10 > > > https://github.com/intel/libva/commit/cf11abe5e1b9c93ee75cf97407695716 > > 2c1605b9 > > BTW, there's a typo in your libva doxygen comments ("berief" <-> "brief"). Did > doxygen even run successfully?
I am not send patches to libva, but I can report it to them. > > [...] > > +check_struct "va/va.h va/va_vpp.h" "VAProcPipelineParameterBuffer" > > +output_hdr_metadata > > Does libva really not do any micro version bumping or the likes when adding > features? currently I am not sure about the micro version changes in libva, but I will keep watch the version bumping. > > > +int ff_vaapi_vpp_make_param_buffers2(AVFilterContext *avctx, > > Shouldn't ff_vaapi_vpp_make_param_buffers() be converted to use this? > Eventually? ff_vaapi_vpp_make_param_buffers doesn't work for this filter. because this filter read metadata from input AVFrame, and ff_vaapi_vpp_make_param_buffers usually is used the filter buffer parameter doesn't change during the VPP. > > > +#define LAV_UINT32(a) (a.num) > > + in_metadata->max_display_mastering_luminance = > > + LAV_UINT32(hdr_meta->max_luminance); > > + in_metadata->min_display_mastering_luminance = > > + LAV_UINT32(hdr_meta->min_luminance); > > +#undef LAV_UINT16 > > What are you undefining here? Didn't you mean to undef LAV_UINT32? Got it , I will fix this. > > > + if (!metadata) { > > + av_log(avctx, AV_LOG_ERROR, "Can't new side data > > + for output\n"); > > "new" is not really a verb. ;-) "Can't create new ..."? ("Cannot" > preferred, actually.) Got it, I will fix this. > > > + if (!metadata_lt) { > > + av_log(avctx, AV_LOG_ERROR, "Can't new side data > > + for output\n"); > > Ditto. Got it, I will fix this. > > > + if ((err = ff_formats_ref(ff_make_format_list(pix_in_fmts), > > + &avctx->inputs[0]->out_formats)) < 0) > > + return err; > > There was some discussion here that the style > err = ff_formats_ref(ff_make_format_list(pix_in_fmts), &avctx->inputs[0]- > >out_formats) > if (err < 0) > return err; > > is preferred for readability. Got it, I will do the change. > > Sorry that some remarks are in form of questions. I'm not totally sure. > > Cheers, > Moritz Thank you for the comments, Zachary > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel