On Wed, Feb 14, 2018 at 10:54:26PM +0000, Mark Thompson wrote:
> --- /dev/null
> +++ b/libavfilter/vaapi_vpp.c
> @@ -0,0 +1,370 @@
> +
> +#include <string.h>
> +
> +#include "config.h"
This does not appear to use config.h.
> +#include "libavutil/avassert.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/pixdesc.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "vaapi_vpp.h"
nit: empty line between header groups
> +int ff_vaapi_vpp_config_input(AVFilterLink *inlink)
> +{
> + ctx->input_frames_ref = av_buffer_ref(inlink->hw_frames_ctx);
> + if (!ctx->input_frames_ref) {
> + av_log(avctx, AV_LOG_ERROR, "A input frames reference create "
> + "failed.\n");
That sentence sounds ungrammatical to me.
> + ctx->input_frames = (AVHWFramesContext*)ctx->input_frames_ref->data;
nit: space before *
> + ctx->device_ref = av_buffer_ref(ctx->input_frames->device_ref);
> + if (!ctx->device_ref) {
> + av_log(avctx, AV_LOG_ERROR, "A device reference create "
> + "failed.\n");
see above
> + ctx->hwctx = ((AVHWDeviceContext*)ctx->device_ref->data)->hwctx;
see above
> +int ff_vaapi_vpp_colour_standard(enum AVColorSpace av_cs)
> +{
> + switch(av_cs) {
switch (
> +#define CS(av, va) case AVCOL_SPC_ ## av: return VAProcColorStandard ## va;
> + CS(BT709, BT709);
> + CS(BT470BG, BT601);
> + CS(SMPTE170M, SMPTE170M);
> + CS(SMPTE240M, SMPTE240M);
> +#undef CS
> + default:
> + return VAProcColorStandardNone;
> + }
> +}
I don't think the #define eases readability, on the contrary.
> +int ff_vaapi_vpp_render_picture(AVFilterContext *avctx,
> + VAProcPipelineParameterBuffer *params,
> + VASurfaceID output_surface)
> +{
> + int err = 0;
> + VAAPIVPPContext *ctx = avctx->priv;
odd extra space
> +void ff_vaapi_vpp_ctx_init(AVFilterContext *avctx)
> +{
> + int i;
> + VAAPIVPPContext *ctx = avctx->priv;
same
> +void ff_vaapi_vpp_ctx_uninit(AVFilterContext *avctx)
> +{
> + VAAPIVPPContext *ctx = avctx->priv;
same
> --- /dev/null
> +++ b/libavfilter/vaapi_vpp.h
> @@ -0,0 +1,79 @@
> +#ifndef AVFILTER_VAAPI_VPP_H
> +#define AVFILTER_VAAPI_VPP_H
> +
> +#include <va/va.h>
> +#include <va/va_vpp.h>
I think this needs to go in SKIPHEADERS.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
