[FFmpeg-devel] [PATCH] added support for hardware assist H264 video encoding for the Raspberry Pi

[Lou Logan]

On Mon, 21 Mar 2016 20:07:01 -0700, Amancio Hasty wrote:

> From 874a72eec2a78f4935fea091003e534b5f8d5413 Mon Sep 17 00:00:00 2001
> From: Amancio Hasty <ahasty at gmail.com>
> Date: Mon, 21 Mar 2016 18:56:05 -0700
> Subject: [PATCH] added support for hardware assist H264  video encoding for
>  the Raspberry Pi
> 
> ---
>  configure              |  12 ++
>  libavcodec/Makefile    |   1 +
>  libavcodec/allcodecs.c |   2 +
>  libavcodec/vc264.c     | 387 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 402 insertions(+)
>  create mode 100644 libavcodec/vc264.c
> 
[...]
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 2a25d66..3c7bd9b 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -74,6 +74,7 @@ void avcodec_register_all(void)
>      initialized = 1;
>  

Nit: Whitespace on the line above should be removed.

[...]
> --- /dev/null
> +++ b/libavcodec/vc264.c
> @@ -0,0 +1,387 @@
> +/*  H.264 hardware assist video encoding code taken from
> + * raspberry's os :
> + *   /opt/vc/src/hello_pi/hello_encode/encode.c
> + */
> +
> +/*
> +Copyright (c) 2012, Broadcom Europe Ltd
> +Copyright (c) 2012, Kalle Vahlman <zuh at iki>
> +                    Tuomas Kulve <tuomas at kulve.fi>
> +All rights reserved.
> +
> +Redistribution and use in source and binary forms, with or without
> +modification, are permitted provided that the following conditions are met:
> +* Redistributions of source code must retain the above copyright
> +      notice, this list of conditions and the following disclaimer.
> +      * Redistributions in binary form must reproduce the above copyright
> +      notice, this list of conditions and the following disclaimer in the
> +      documentation and/or other materials provided with the distribution.
> +      * Neither the name of the copyright holder nor the
> +      names of its contributors may be used to endorse or promote products
> +      derived from this software without specific prior written permission.
> +
> +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
> +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY
> +DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> +(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> +LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> +ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> +SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

I wonder if any of the above legalese is compatible. Granted, I see a
similar paragraph in "libavformat/aadec.c".

> + * ffmpeg driver for hardware assist video H.264 encoding using Broadcom's GPU
> + * Copyright (C) 2016 Amancio Hasty ahasty at gmail.com
> + *
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +
> +/**
> + * @ file vc264.c
> + * Broadcom bm2865's Visual Core hardware assist h264 using
> +   openMax interface to the GPU.
> +
> +*/
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#define OMX_SKIP64BIT
> +#include "bcm_host.h"
> +#include "ilclient.h"
> +#include "avcodec.h"
> +#include "internal.h"
> +
> +typedef struct VC264Context {
> +  OMX_VIDEO_PARAM_PORTFORMATTYPE format;
> +  OMX_PARAM_PORTDEFINITIONTYPE def;
> +  COMPONENT_T *video_encode;
> +  COMPONENT_T *list[5];
> +  OMX_BUFFERHEADERTYPE *buf;
> +  OMX_BUFFERHEADERTYPE *out;
> +  ILCLIENT_T *client;
> +  OMX_VIDEO_PARAM_BITRATETYPE bitrateType;
> +  int  width;
> +  int height;
> +  int bit_rate;
> +} VC264Context;
> +
> +
> +static int vc264_init(AVCodecContext *avctx) {
> +
> +
> +
> +   OMX_ERRORTYPE r;
> +   int error;
> +
> +
> +
> +   VC264Context *vc = avctx->priv_data;
> +
> +   vc->width = avctx->width;
> +   vc->height = avctx->height;
> +   vc->bit_rate = avctx->bit_rate;
> +   printf("vc264: bit rate %d \n", avctx->bit_rate);
> +#if FF_API_CODED_FRAME
> +FF_DISABLE_DEPRECATION_WARNINGS
> +
> +   avctx->coded_frame = av_frame_alloc();
> +   avctx->coded_frame->pict_type = AV_PICTURE_TYPE_I;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +
> +
> +   memset(&vc->list, 0, sizeof(vc->list));
> +   bcm_host_init();
> +   if ((vc->client = ilclient_init()) == NULL) {
> +      return -3;
> +   }
> +   error = OMX_Init();
> +
> +   if (error  != OMX_ErrorNone) {
> +      ilclient_destroy(vc->client);
> +      av_log(avctx,AV_LOG_ERROR,"in vc264_init OMX_Init failed ");
> +     return -4;
> +    }
> +
> +   // create video_encode
> +   r = ilclient_create_component(vc->client, &vc->video_encode, (char *) "video_encode",
> +				 ILCLIENT_DISABLE_ALL_PORTS |
> +				 ILCLIENT_ENABLE_INPUT_BUFFERS |
> +				 ILCLIENT_ENABLE_OUTPUT_BUFFERS);

Tabs should be converted to spaces. There are many instances of tabs
being used in this patch.

Others will have to provide a more technical review (not to mention
possible additions docs, Changelog, MAINTAINERS, and
libavcodec/version.h).