Re: [libav-devel] [PATCH 3/3] Add initial tidsp support for MPEG-4

2012-02-26 Thread Felipe Contreras
On Sun, Feb 26, 2012 at 7:54 PM, Felipe Contreras
 wrote:
> Signed-off-by: Felipe Contreras 

> +       for (i = 0; i < avctx->height; i++)
> +               memcpy(f->data[0] + i * f->linesize[0], p1 + i * 
> avctx->width, avctx->width);
> +       for (i = 0; i < avctx->height / 2; i++)
> +               memcpy(f->data[1] + i * f->linesize[1], p2 + i * avctx->width 
> / 2, avctx->width / 2);
> +       for (i = 0; i < avctx->height / 2; i++)
> +               memcpy(f->data[2] + i * f->linesize[2], p3 + i * avctx->width 
> / 2, avctx->width / 2);

Is there a way to avoid this? Say, if the client requests a single plane.

Cheers.

-- 
Felipe Contreras
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 3/3] Add initial tidsp support for MPEG-4

2012-02-26 Thread Diego Biurrun
On Sun, Feb 26, 2012 at 07:54:26PM +0200, Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras 
> ---
>  configure|5 ++
>  libavcodec/Makefile  |1 +
>  libavcodec/allcodecs.c   |1 +
>  libavcodec/tidsp_mpeg4.c |  156 
> ++
>  4 files changed, 163 insertions(+)
>  create mode 100644 libavcodec/tidsp_mpeg4.c

Please also add a Changelog entry.

> --- /dev/null
> +++ b/libavcodec/tidsp_mpeg4.c
> @@ -0,0 +1,156 @@
> +/*
> + * MPEG-4 / H.263 HW decode acceleration through TI DSP
> + *
> + * This file is part of FFmpeg.
> + *
> + * This file may be used under the terms of the GNU Lesser General Public
> + * License version 2.1, a copy of which is found in COPYING.LGPLv2.1 included
> + * in the packaging of this file.
> + */

Please employ our standard license boilerplate for LGPL 2.1 or later.

> +static void handle_buffer(struct td_context *td_ctx, struct td_buffer *b)
> +{
> + AVCodecContext *avctx = td_ctx->client;
> + struct td_av_context *ctx = avctx->hwaccel_private;
> +
> + if (b->port->id == 0)
> + return;
> + if (ctx->output_buffer)
> + av_log(avctx, AV_LOG_ERROR, "overriding buffer\n");
> + ctx->output_buffer = b;
> +}

Replace all tabs by spaces.

> +static av_cold int init(AVCodecContext *avctx)
> +{
> + struct td_av_context *ctx;
> + struct td_context *td_ctx;
> +
> + avctx->hwaccel_private = ctx = av_mallocz(sizeof(*ctx));
> + ctx->td_ctx = td_ctx = td_new(avctx);

That malloc looks unchecked to me.

> + td_ctx->codec = &td_mp4vdec_codec;
> + td_ctx->width = avctx->width;
> + td_ctx->height = avctx->height;
> + td_ctx->handle_buffer = handle_buffer;

nit: please vertically align the '='.

> +static int start_frame(AVCodecContext *avctx, const uint8_t *buffer, 
> uint32_t size)

nit: please break this long line.

> + ctx->output_buffer = NULL;
> + f = (AVFrame*) &s->current_picture;

Wouldn't

  f = &s->current_picture.f;

work as well?

> + for (i = 0; i < avctx->height; i++)
> + memcpy(f->data[0] + i * f->linesize[0], p1 + i * avctx->width, 
> avctx->width);
> + for (i = 0; i < avctx->height / 2; i++)
> + memcpy(f->data[1] + i * f->linesize[1], p2 + i * avctx->width / 
> 2, avctx->width / 2);
> + for (i = 0; i < avctx->height / 2; i++)
> + memcpy(f->data[2] + i * f->linesize[2], p3 + i * avctx->width / 
> 2, avctx->width / 2);

nit: long lines

> +AVHWAccel ff_mpeg4_tidsp_hwaccel = {
> + .name = "mpeg4_tidsp",
> + .type = AVMEDIA_TYPE_VIDEO,
> + .id = CODEC_ID_MPEG4,
> + .pix_fmt = PIX_FMT_YUV420P,
> + .start_frame = start_frame,
> + .end_frame = end_frame,
> + .decode_slice = decode_slice,
> + .init = init,
> + .close = close,

Please align the '='.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel