[FFmpeg-devel] [PATCH v3 2/4] lavc/avs3_paeser: add avs3 parser
hwren
hwrenx at 126.com
Mon Aug 17 14:35:14 EEST 2020
Fix the mail format of reply message from <hbj515 at sina.com> and reset it to the correct thread.
same content.
At 2020-08-12 05:43:24, "Mark Thompson" <sw at jkqxz.net> wrote:
>
>Typo "paeser" in the title.
>
>On 05/08/2020 17:18, hwrenx at 126.com wrote:
>> From: hwren <hwrenx at 126.com>
>>
>> Signed-off-by: hbj <hanbj at pku.edu.cn>
>> Signed-off-by: hwren <hwrenx at 126.com>
>> ---
>> libavcodec/Makefile | 1 +
>> libavcodec/avs3_parser.c | 184 +++++++++++++++++++++++++++++++++++++++
>> libavcodec/parsers.c | 1 +
>> 3 files changed, 186 insertions(+)
>> create mode 100644 libavcodec/avs3_parser.c
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index 5a6ea59715..f1512779be 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -1053,6 +1053,7 @@ OBJS-$(CONFIG_AC3_PARSER) += ac3tab.o aac_ac3_parser.o
>> OBJS-$(CONFIG_ADX_PARSER) += adx_parser.o adx.o
>> OBJS-$(CONFIG_AV1_PARSER) += av1_parser.o av1_parse.o
>> OBJS-$(CONFIG_AVS2_PARSER) += avs2_parser.o
>> +OBJS-$(CONFIG_AVS3_PARSER) += avs3_parser.o
>> OBJS-$(CONFIG_BMP_PARSER) += bmp_parser.o
>> OBJS-$(CONFIG_CAVSVIDEO_PARSER) += cavs_parser.o
>> OBJS-$(CONFIG_COOK_PARSER) += cook_parser.o
>> diff --git a/libavcodec/avs3_parser.c b/libavcodec/avs3_parser.c
>> new file mode 100644
>> index 0000000000..7a7f826c59
>> --- /dev/null
>> +++ b/libavcodec/avs3_parser.c
>> @@ -0,0 +1,184 @@
>> +/*
>> + * AVS3-P2/IEEE1857.10 video parser.
>> + * Copyright (c) 2020 Zhenyu Wang <wangzhenyu at pkusz.edu.cn>
>> + * Bingjie Han <hanbj at pkusz.edu.cn>
>> + * 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 "parser.h"
>> +
>> +#define SLICE_MAX_START_CODE 0x000001af
>> +
>> +#define ISPIC(x) ((x) == 0xB3 || (x) == 0xB6)
>> +#define ISUNIT(x) ((x) == 0xB0 || ISPIC(x))
>
>Perhaps give the magic numbers here symbolic constants? (Sequence header, etc.)
Thanks, we will define the constants as bellow:
#define AVS3_VIDEO_SEQ_START_CODE 0xB0
#define AVS3_INTRA_PIC_START_CODE 0xB3
#define AVS3_INTER_PIC_START_CODE 0xB6
>
>> +
>> +static av_cold int avs3_find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size)
>
>This function doesn't look cold.
Sorry, the av_cold will be removed.
>
>> +{
>> + int pic_found = pc->frame_start_found;
>> + uint32_t state = pc->state;
>> + int cur = 0;
>> +
>> + if (!pic_found) {
>> + for (; cur < buf_size; ++cur) {
>> + state = (state<<8) | buf[cur];
>> + if (ISPIC(buf[cur])){
>> + ++cur;
>> + pic_found = 1;
>> + break;
>> + }
>> + }
>> + }
>> +
>> + if (pic_found) {
>> + if (!buf_size)
>> + return END_NOT_FOUND;
>> + for (; cur < buf_size; ++cur) {
>> + state = (state << 8) | buf[cur];
>> + if ((state & 0xFFFFFF00) == 0x100 && ISUNIT(state & 0xFF)) {
>> + pc->frame_start_found = 0;
>> + pc->state = -1;
>> + return cur - 3;
>> + }
>> + }
>> + }
>> +
>> + pc->frame_start_found = pic_found;
>> + pc->state = state;
>> +
>> + return END_NOT_FOUND;
>> +}
>> +
>> +static unsigned int read_bits(const char** ppbuf, int *pidx, int bits)
>> +{
>> + const char* p = *ppbuf;
>> + int idx = *pidx;
>> + unsigned int val = 0;
>> +
>> + while (bits) {
>> + bits--;
>> + val = (val << 1) | (((*p) >> idx) & 0x1);
>> + if (--idx < 0) {
>> + idx = 7;
>> + p++;
>> + }
>> + }
>> +
>> + *ppbuf = p;
>> + *pidx = idx;
>> +
>> + return val;
>> +}
>> +
>> +static void parse_avs3_nal_units(AVCodecParserContext *s, const uint8_t *buf,
>> + int buf_size, AVCodecContext *avctx)
>> +{
>> + if (buf_size < 5) {
>> + return;
>> + }
>> +
>> + if (buf[0] == 0x0 && buf[1] == 0x0 && buf[2] == 0x1) {
>> + if (buf[3] == 0xB0) {
>> + static const int avs3_fps_num[9] = {0, 240000, 24, 25, 30000, 30, 50, 60000, 60 };
>> + static const int avs3_fps_den[9] = {1, 1001, 1, 1, 1001, 1, 1, 1001, 1 };
>
>Is this meant to be the MPEG-1 frame rate code table? (It looks the same other than num[1], which looks like a typo.) If so, there is a global ff_mpeg12_frame_rate_tab already containing the values.
>
The AVS3 frame rate code table is not same to the MPEG-1, we will use the next table:
const AVRational avs3_frame_rate_tab[16] = {
{ 0 , 0 }, // forbid
{ 24000, 1001},
{ 24 , 1 },
{ 25 , 1 },
{ 30000, 1001},
{ 30 , 1 },
{ 50 , 1 },
{ 60000, 1001},
{ 60 , 1 },
{ 100 , 1 },
{ 120 , 1 },
{ 200 , 1 },
{ 240 , 1 },
{ 300 , 1 },
{ 0 , 0 }, // reserved
{ 0 , 0 } // reserved
};
>> + int profile,ratecode;
>> + const char* p = buf + 4;
>> + int idx = 7;
>> +
>> + s->key_frame = 1;
>> + s->pict_type = AV_PICTURE_TYPE_I;
>> +
>> + profile = read_bits(&p, &idx, 8);
>> + // level(8) + progressive(1) + field(1) + library(2) + resv(1) + width(14) + resv(1) + height(14) + chroma(2) + sampe_precision(3)
>> + read_bits(&p, &idx, 47);
>
>This ad-hoc reimplementation of get_bits does not seem like a good idea - it's reading one bit at a time and lacks any checks for overflow. Please just use get_bits().
>
Thanks for your advice, we will use get_bits() in the next version.
>> +
>> + if (profile == 0x22) {
>> + avctx->pix_fmt = read_bits(&p, &idx, 3) == 1 ? AV_PIX_FMT_YUV420P : AV_PIX_FMT_YUV420P10LE;
>> + }
>
>Are other values of profile likely to be valid in future, requiring different parsing? Is it worth rejecting these here?
Will be fixed. Thanks.
>
>> +
>> + // resv(1) + aspect(4)
>> + read_bits(&p, &idx, 5);
>> +
>> + ratecode = read_bits(&p, &idx, 4);
>> +
>> + // resv(1) + bitrate_low(18) + resv(1) + bitrate_high(12)
>> + read_bits(&p, &idx, 32);
>> +
>> + avctx->has_b_frames = !read_bits(&p, &idx, 1);
>> +
>> + avctx->framerate.num = avctx->time_base.den = avs3_fps_num[ratecode];
>> + avctx->framerate.den = avctx->time_base.num = avs3_fps_den[ratecode];
>
>ratecode isn't checked, so might overflow the array.
ratecode is in range of [0, 15], it will not be overflowed for the new table avs3_frame_rate_tab.
>
>> +
>> + s->width = s->coded_width = avctx->width;
>> + s->height = s->coded_height = avctx->height;
>> +
>> + av_log(avctx, AV_LOG_DEBUG,
>> + "avs3 parse seq hdr: profile %d; coded wxh: %dx%d; "
>> + "frame_rate_code %d\n", profile, avctx->width, avctx->height, ratecode);
>> +
>> + } else if (buf[3] == 0xB3) {
>> + s->key_frame = 1;
>> + s->pict_type = AV_PICTURE_TYPE_I;
>> + } else if (buf[3] == 0xB6){
>> + s->key_frame = 0;
>> + if (buf_size > 9) {
>> + int pic_code_type = buf[8] & 0x3;
>> + if (pic_code_type == 1 || pic_code_type == 3) {
>> + s->pict_type = AV_PICTURE_TYPE_P;
>> + } else {
>> + s->pict_type = AV_PICTURE_TYPE_B;
>> + }
>> + }
>> + }
>
>What happens if you don't get a sequence header before seeing some frames?
Even if the sequence header is not got in the first several frames, it will be decoded again in the next decoding process in libuavs3d.c.
>
>> + }
>> +}
>> +
>> +
>> +static int avs3_parse(AVCodecParserContext *s, AVCodecContext *avctx,
>> + const uint8_t **poutbuf, int *poutbuf_size,
>> + const uint8_t *buf, int buf_size)
>> +{
>> + ParseContext *pc = s->priv_data;
>> + int next;
>> +
>> + if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) {
>> + next = buf_size;
>> + } else {
>> + next = avs3_find_frame_end(pc, buf, buf_size);
>> + if (ff_combine_frame(pc, next, &buf, &buf_size) < 0) {
>> + *poutbuf = NULL;
>> + *poutbuf_size = 0;
>> + return buf_size;
>> + }
>> + }
>> +
>> + parse_avs3_nal_units(s, buf, buf_size, avctx);
>> +
>> + *poutbuf = buf;
>> + *poutbuf_size = buf_size;
>> +
>> + return next;
>> +}
>> +
>> +AVCodecParser ff_avs3_parser = {
>> + .codec_ids = { AV_CODEC_ID_AVS3 },
>> + .priv_data_size = sizeof(ParseContext),
>> + .parser_parse = avs3_parse,
>> + .parser_close = ff_parse_close,
>> + .split = ff_mpeg4video_split,
>> +};
>> diff --git a/libavcodec/parsers.c b/libavcodec/parsers.c
>> index 7d75cea830..aab1d5e3e5 100644
>> --- a/libavcodec/parsers.c
>> +++ b/libavcodec/parsers.c
>> @@ -28,6 +28,7 @@ extern AVCodecParser ff_ac3_parser;
>> extern AVCodecParser ff_adx_parser;
>> extern AVCodecParser ff_av1_parser;
>> extern AVCodecParser ff_avs2_parser;
>> +extern AVCodecParser ff_avs3_parser;
>> extern AVCodecParser ff_bmp_parser;
>> extern AVCodecParser ff_cavsvideo_parser;
>> extern AVCodecParser ff_cook_parser;
>>
>
>- Mark
>_______________________________________________
>ffmpeg-devel mailing list
>ffmpeg-devel at ffmpeg.org
>https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>To unsubscribe, visit link above, or email
>ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list