Quoting Diego Biurrun (2017-08-12 10:10:30)
> On Fri, Aug 11, 2017 at 09:46:17AM +0800, Huang, Zhengxu wrote:
> > + s->surface_ptrs_out =
> > av_mallocz_array(out_frames_hwctx->nb_surfaces,
> > +
> > sizeof(*s->surface_ptrs_out));
> > + if (!s->surface_ptrs_out) {
> > + av_buffer_unref(&out_frames_ref);
> > + return AVERROR(ENOMEM);
> > + }
> > +
> > + for (i = 0; i < out_frames_hwctx->nb_surfaces; i++)
> > + s->surface_ptrs_out[i] = out_frames_hwctx->surfaces + i;
> > + s->nb_surface_ptrs_out = out_frames_hwctx->nb_surfaces;
> > +
> > + av_buffer_unref(&outlink->hw_frames_ctx);
> > + outlink->hw_frames_ctx = out_frames_ref;
> > + } else
> > + s->out_mem_mode = MFX_MEMTYPE_SYSTEM_MEMORY;
> > +
> > + /* extract the properties of the "master" session given to us */
> > + ret = MFXQueryIMPL(device_hwctx->session, &impl);
> > + if (ret == MFX_ERR_NONE)
> > + ret = MFXQueryVersion(device_hwctx->session, &ver);
> > + if (ret != MFX_ERR_NONE) {
> > + av_log(avctx, AV_LOG_ERROR, "Error querying the session
> > attributes\n");
> > + return AVERROR_UNKNOWN;
> > + }
>
> Using plain "else" instead of another if would be clearer.
That's not the same thing as what the code does now. Read it more
carefully.
>
> > + /* create a "slave" session with those same properties, to be used for
> > vpp*/
>
> space before *
>
> > +int ff_qsvvpp_create(AVFilterContext *avctx, QSVVPPContext **vpp,
> > QSVVPPParam *param)
> > +{
> > + int ret = 0, i;
>
> The init is pointless.
>
> > + QSVVPPContext *s;
> > +
> > + s = av_mallocz(sizeof(*s));
> > + if (!s) {
> > + ret = AVERROR(ENOMEM);
> > + goto failed;
> > + }
>
> No need for the goto here, you can just return directly.
>
> > + /* Init each input's information*/
>
> space before *
>
> > +failed:
> > + if (s)
> > + ff_qsvvpp_free(&s);
> > +
> > + return ret;
> > +}
>
> The if is unnecessary if you dropped the goto above.
It's actually unnecessary in all cases, since ff_qsvvpp_free() is a
no-op on NULL.
>
> > +int ff_qsvvpp_filter_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame
> > *picref)
> > +{
> > + in_frame = submit_frame(s, inlink, picref);
> > + if (!in_frame) {
> > + av_log(ctx, AV_LOG_ERROR, "Fail to submit frame on input[%d]\n",
> > + FF_INLINK_IDX(inlink));
>
> "Failed", indentation is off.
>
> > + return AVERROR(EINVAL);
>
> Is this an error due to user-supplied data or user-supplied configuration?
> Because that's what EINVAL is for.
Should be ENOMEM
>
> Don't you leak vpp->comp_conf.InputStream here?
>
No, uninit is always called even if init fails.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
