[FFmpeg-devel] [PATCH] lavc,lavf: add libdavs2 decoder
Mark Thompson
sw at jkqxz.net
Sat May 26 17:28:27 EEST 2018
On 26/05/18 05:41, hwren wrote:
> Add Chinese AVS2 video decoder, FFmpeg can make use of the libdavs2 library for AVS2 decoding.
>
> Signed-off-by: hwren <hwrenx at 126.com>
> ---
> Changelog | 1 +
> configure | 6 ++
> doc/general.texi | 8 ++
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/avcodec.h | 1 +
> libavcodec/codec_desc.c | 7 ++
> libavcodec/libdavs2.c | 216 ++++++++++++++++++++++++++++++++++++++++++++++++
> libavformat/riff.c | 1 +
> 9 files changed, 242 insertions(+)
> create mode 100644 libavcodec/libdavs2.c
>
> diff --git a/Changelog b/Changelog
> index 3d25564..0f97679 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -9,6 +9,7 @@ version <next>:
> - aderivative and aintegral audio filters
> - pal75bars and pal100bars video filter sources
> - support mbedTLS based TLS
> +- AVS2 video decoder
Note the library that is used here. (This isn't a native decoder, it requires an external library.)
>
>
> version 4.0:
> diff --git a/configure b/configure
> index 09ff0c5..9aec00d 100755
> --- a/configure
> +++ b/configure
> @@ -276,6 +276,7 @@ External library support:
> --enable-libx264 enable H.264 encoding via x264 [no]
> --enable-libx265 enable HEVC encoding via x265 [no]
> --enable-libxavs enable AVS encoding via xavs [no]
> + --enable-libdavs2 enable AVS2 decoding via davs2 [no]
This list should stay in alphabetical order.
> --enable-libxcb enable X11 grabbing using XCB [autodetect]
> --enable-libxcb-shm enable X11 grabbing shm communication [autodetect]
> --enable-libxcb-xfixes enable X11 grabbing mouse rendering [autodetect]
> @@ -1641,6 +1642,7 @@ EXTERNAL_LIBRARY_GPL_LIST="
> libx264
> libx265
> libxavs
> + libdavs2
Likewise this one, and more below.
> libxvid
> "
>
> @@ -3095,6 +3097,7 @@ libx264rgb_encoder_deps="libx264 x264_csp_bgr"
> libx264rgb_encoder_select="libx264_encoder"
> libx265_encoder_deps="libx265"
> libxavs_encoder_deps="libxavs"
> +libdavs2_decoder_deps="libdavs2"
> libxvid_encoder_deps="libxvid"
> libzvbi_teletext_decoder_deps="libzvbi"
> vapoursynth_demuxer_deps="vapoursynth"
> @@ -6104,6 +6107,9 @@ enabled libx264 && { check_pkg_config libx264 x264 "stdint.h x264.h" x
> enabled libx265 && require_pkg_config libx265 x265 x265.h x265_api_get &&
> require_cpp_condition x265.h "X265_BUILD >= 68"
> enabled libxavs && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
> +enabled libdavs2 && require_pkg_config libdavs2 davs2 "stdint.h davs2.h" davs2_decoder_decode &&
That shouldn't need stdint.h? If it does, then the library should probably be fixed to include it correctly.
> + { check_cpp_condition davs2 davs2.h "DAVS2_BUILD >= 12" ||
> + die "ERROR: libdavs2 version must be >= 12." ; }
Can you check the version with pkgconfig instead?
> enabled libxvid && require libxvid xvid.h xvid_global -lxvidcore
> enabled libzimg && require_pkg_config libzimg "zimg >= 2.7.0" zimg.h zimg_get_api_version
> enabled libzmq && require_pkg_config libzmq libzmq zmq.h zmq_ctx_new
> diff --git a/doc/general.texi b/doc/general.texi
> index 2583006..d3c1503 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -17,6 +17,14 @@ for more formats. None of them are used by default, their use has to be
> explicitly requested by passing the appropriate flags to
> @command{./configure}.
>
> + at section libdavs2
> +
> +FFmpeg can make use of the libdavs2 library for AVS2 decoding.
> +
> +Go to @url{https://github.com/pkuvcl/davs2} and follow the instructions for
> +installing the library. Then pass @code{--enable-libdavs2} to configure to
> +enable it.
> +
> @section Alliance for Open Media libaom
>
> FFmpeg can make use of the libaom library for AV1 decoding.
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 3ab071a..e85b74c 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -985,6 +985,7 @@ OBJS-$(CONFIG_LIBX262_ENCODER) += libx264.o
> OBJS-$(CONFIG_LIBX264_ENCODER) += libx264.o
> OBJS-$(CONFIG_LIBX265_ENCODER) += libx265.o
> OBJS-$(CONFIG_LIBXAVS_ENCODER) += libxavs.o
> +OBJS-$(CONFIG_LIBDAVS2_DECODER) += libdavs2.o
> OBJS-$(CONFIG_LIBXVID_ENCODER) += libxvid.o
> OBJS-$(CONFIG_LIBZVBI_TELETEXT_DECODER) += libzvbi-teletextdec.o ass.o
>
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 7b7a8c7..6103081 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -705,6 +705,7 @@ extern AVCodec ff_libx264_encoder;
> extern AVCodec ff_libx264rgb_encoder;
> extern AVCodec ff_libx265_encoder;
> extern AVCodec ff_libxavs_encoder;
> +extern AVCodec ff_libdavs2_decoder;
> extern AVCodec ff_libxvid_encoder;
> extern AVCodec ff_libzvbi_teletext_decoder;
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index fb0c6fa..a8cbc7b 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -303,6 +303,7 @@ enum AVCodecID {
> AV_CODEC_ID_KMVC,
> AV_CODEC_ID_FLASHSV,
> AV_CODEC_ID_CAVS,
> + AV_CODEC_ID_AVS2,
This needs to go at the end of the list. (Changing existing values breaks ABI.)
This change and the one to add the descriptor should be a separate patch, and include an APIchanges entry because they modify an installed header.
> AV_CODEC_ID_JPEG2000,
> AV_CODEC_ID_VMNC,
> AV_CODEC_ID_VP5,
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 79552a9..9b50dae 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -651,6 +651,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
> .props = AV_CODEC_PROP_LOSSY | AV_CODEC_PROP_REORDER,
> },
> {
> + .id = AV_CODEC_ID_AVS2,
> + .type = AVMEDIA_TYPE_VIDEO,
> + .name = "avs2",
> + .long_name = NULL_IF_CONFIG_SMALL("Chinese AVS2 (Audio Video Standar) (AVS2-P2, JiZhun profile)"),
> + .props = AV_CODEC_PROP_LOSSY,
> + },
> + {
> .id = AV_CODEC_ID_JPEG2000,
> .type = AVMEDIA_TYPE_VIDEO,
> .name = "jpeg2000",
> diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
> new file mode 100644
> index 0000000..3537f40
> --- /dev/null
> +++ b/libavcodec/libdavs2.c
> @@ -0,0 +1,216 @@
> +/*
> + * AVS2 decoding using the davs2 library
> + *
> + * Copyright (C) 2018 Yiqun Xu, <yiqun.xu at vipl.ict.ac.cn>
> + * Falei Luo, <falei.luo at gmail.com>
> + * Huiwen Ren, <hwrenx 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
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/common.h"
> +#include "libavutil/avutil.h"
> +#include "avcodec.h"
> +#include "libavutil/imgutils.h"
> +#include "internal.h"
> +
> +#include <stdio.h>
> +#include <stdlib.h>
These two headers aren't used?
> +#include <davs2.h>
> +
> +typedef struct DAVS2Context {
> + AVCodecContext *avctx;
> + void *dec_handle;
> + int got_seqhdr;
> +
> + void *decoder;
> +
> + AVFrame *frame;
> + davs2_param_t param; // decoding parameters
> + davs2_packet_t packet; // input bitstream
> + int ret;
> +
> + int img_width[3];
> + int img_height[3];
> + int out_flag;
> + int decoded_frames;
> +
Don't add trailing spaces. (Also more below.)
> + davs2_picture_t out_frame; // output data, frame data
> + davs2_seq_info_t headerset; // output data, sequence header
> +
> +}DAVS2Context;
Some of the fields of this structure look like they should be on the stack in functions below instead.
> +
> +static int ff_davs2_init(AVCodecContext *avctx)
> +{
> + DAVS2Context *cad = avctx->priv_data;
> +
> + /* init the decoder */
> + cad->param.threads = 0;
> + cad->param.i_info_level = 0;
> + cad->decoder = davs2_decoder_open(&cad->param);
> + avctx->flags |= AV_CODEC_FLAG_TRUNCATED;
> +
> + av_log(avctx, AV_LOG_WARNING, "[davs2] decoder created. 0x%llx\n", cad->decoder);
This shouldn't be a warning. Maybe VERBOSE?
void* can't be printed with "%llx", if you want the pointer in this log line then "%p".
> + return 0;
> +}
> +
> +/* ---------------------------------------------------------------------------
> + */
> +static int DumpFrames(AVCodecContext *avctx, davs2_picture_t *pic, davs2_seq_info_t *headerset, AVFrame *frm)
> +{
> + DAVS2Context *cad = avctx->priv_data;
> + avctx->flags |= AV_CODEC_FLAG_TRUNCATED;
Why this line?
> +
> + if (headerset == NULL) {
It's a pointer, just "!headerset".
> + return 0;
> + }
> +
> + if (pic == NULL || pic->ret_type == DAVS2_GOT_HEADER) {
> + avctx->frame_size = (headerset->horizontal_size * headerset->vertical_size * 3 * headerset->bytes_per_sample) >> 1;
frame_size isn't used for video.
> + avctx->coded_width = headerset->horizontal_size;
> + avctx->coded_height = headerset->vertical_size;
> + avctx->width = headerset->horizontal_size;
> + avctx->height = headerset->vertical_size;
Must the coded size and actual size really be equal?
> + avctx->pix_fmt = (headerset->output_bitdepth == 10 ? AV_PIX_FMT_YUV420P10 : AV_PIX_FMT_YUV420P);
> + avctx->framerate.num = (int)(headerset->frame_rate);
> + avctx->framerate.den = 1;
frame_rate is a float in the API - maybe use av_d2q() to make this so that it supports non-integer rates?
> + return 0;
> + }
> +
> + const int bytes_per_sample = pic->bytes_per_sample;
> + int i;
Mixed code and declarations are not allowed.
> +
> + for (i = 0; i < 3; ++i) {
> + int size_plane = pic->width[i] * pic->lines[i] * bytes_per_sample;
> + frm->buf[i] = av_buffer_alloc(size_plane);
Allocations must be checked.
> + frm->data[i] = frm->buf[i]->data;
> + frm->linesize[i] = pic->width[i] * bytes_per_sample;
> + memcpy(frm->data[i], pic->planes[i], size_plane);
This is the same format as libavcodec wants, so is there any way to keep a reference to it so we can avoid this copy?
> + }
> +
> + frm->width = cad->headerset.horizontal_size;
> + frm->height = cad->headerset.vertical_size;
> + frm->pts = cad->out_frame.pts;
> +
> + frm->key_frame = 1;
> + frm->pict_type = AV_PICTURE_TYPE_I;
I don't think this is an intra-only codec. If you don't have any way to determine the picture type then just don't set these fields.
> + frm->format = avctx->pix_fmt;
> + cad->out_flag = 1;
> +
> + cad->decoded_frames++;
> + return 1;
> +}
> +
> +static int ff_davs2_end(AVCodecContext *avctx)
> +{
> + DAVS2Context *cad = avctx->priv_data;
> +
> + /* close the decoder */
> + if (cad->decoder != NULL) {
"if (cad->decoder) {"
> + davs2_decoder_close(cad->decoder);
> + av_log(avctx, AV_LOG_WARNING, "[davs2] decoder destroyed. 0x%llx; frames %d\n", cad->decoder, cad->decoded_frames);
Same comments as on the above log line.
> + cad->decoder = NULL;
> + }
> +
> + return 0;
> +}
> +
> +static int davs2_decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPacket *avpkt)
> +{
> + DAVS2Context *cad = avctx->priv_data;
> + int buf_size = avpkt->size;
> + const uint8_t *buf_ptr = avpkt->data;
> + AVFrame *frm = data;
> +
> + *got_frame = 0;
> + cad->out_flag=0;
> + cad->frame = frm;
What is this trying to do? cad->frame is never referenced anywhere else.
> + avctx->flags |= AV_CODEC_FLAG_TRUNCATED;
> +
> + if (buf_size == 0) {
> + cad->packet.data = buf_ptr;
> + cad->packet.len = buf_size;
> + cad->packet.pts = avpkt->pts;
> + cad->packet.dts = avpkt->dts;
> +
> + for (;;) {
> + cad->ret = davs2_decoder_flush(cad->decoder, &cad->headerset, &cad->out_frame);
> +
> + if (cad->ret < 0) {
> + return 0;
> + }
> +
> + if (cad->out_frame.ret_type != DAVS2_DEFAULT) {
> + *got_frame = DumpFrames(avctx, &cad->out_frame, &cad->headerset, frm);
> + davs2_decoder_frame_unref(cad->decoder, &cad->out_frame);
> + }
> + if(cad->out_flag == 1) {
> + break;
out_flag seems to have exactly the same meaning as *got_frame - maybe you can remove it?
> + }
> + }
> + return 0;
> + } else {
> + for(; buf_size > 0;) {
> + int len = buf_size; // for API-3, pass all data in
> +
> + cad->packet.marker = 0;
> + cad->packet.data = buf_ptr;
> + cad->packet.len = len;
> + cad->packet.pts = avpkt->pts;
> + cad->packet.dts = avpkt->dts;
> +
> + len = davs2_decoder_decode(cad->decoder, &cad->packet, &cad->headerset, &cad->out_frame);
> +
> + if (cad->out_frame.ret_type != DAVS2_DEFAULT) {
> + *got_frame = DumpFrames(avctx, &cad->out_frame, &cad->headerset, frm);
> + davs2_decoder_frame_unref(cad->decoder, &cad->out_frame);
> + }
> +
> + if (len < 0) {
> + av_log(NULL, AV_LOG_ERROR, "An decoder error counted\n");
The log line should be the AVCodecContext as the logging context.
Can you get any more detail about the error?
> +
> + return -1;
Does this require any cleanup?
It should return an AVERROR() code rather than -1.
> + }
> +
> + buf_ptr += len;
> + buf_size -= len;
Can you explain how the consuming partial packets works? It looks like if this consumes part of a packet and they returns a frame, the rest of the packet will be discarded (because the next call will be given a new packet).
> +
> + if(cad->out_flag == 1) {
> + break;
> + }
> + }
> + }
> +
> + buf_size = (buf_ptr - avpkt->data);
> +
> + return buf_size;
> +}
> +
> +AVCodec ff_libdavs2_decoder = {
> + .name = "libdavs2",
> + .long_name = NULL_IF_CONFIG_SMALL("Decoder for Chinese AVS2"),
> + .type = AVMEDIA_TYPE_VIDEO,
> + .id = AV_CODEC_ID_AVS2,
> + .priv_data_size = sizeof(DAVS2Context),
> + .init = ff_davs2_init,
> + .close = ff_davs2_end,
> + .decode = davs2_decode_frame,
> + .capabilities = AV_CODEC_CAP_DELAY,//AV_CODEC_CAP_DR1 |
> + .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV420P10,
> + AV_PIX_FMT_NONE },
> +};
> diff --git a/libavformat/riff.c b/libavformat/riff.c
> index 8911725..4153372 100644
> --- a/libavformat/riff.c
> +++ b/libavformat/riff.c
> @@ -369,6 +369,7 @@ const AVCodecTag ff_codec_bmp_tags[] = {
> { AV_CODEC_ID_ZMBV, MKTAG('Z', 'M', 'B', 'V') },
> { AV_CODEC_ID_KMVC, MKTAG('K', 'M', 'V', 'C') },
> { AV_CODEC_ID_CAVS, MKTAG('C', 'A', 'V', 'S') },
> + { AV_CODEC_ID_AVS2, MKTAG('A', 'V', 'S', '2') },
> { AV_CODEC_ID_JPEG2000, MKTAG('m', 'j', 'p', '2') },
> { AV_CODEC_ID_JPEG2000, MKTAG('M', 'J', '2', 'C') },
> { AV_CODEC_ID_JPEG2000, MKTAG('L', 'J', '2', 'C') },
>
This hunk looks like it should be a separate patch?
- Mark
More information about the ffmpeg-devel
mailing list