>On 08/12/2020 05:43, s...@jkqxz.net wrote:>Typo "paeser" in the title.>On
>05/08/2020 17:18, hwr...@126.com wrote:>> From: hwren <hwr...@126.com>>> >>
>Signed-off-by: hbj <ha...@pku.edu.cn>>> Signed-off-by: hwren
><hwr...@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 <wangzhe...@pkusz.edu.cn>>>
>+ * Bingjie Han <ha...@pkusz.edu.cn>>> + * Huiwen Ren <hwr...@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?>> +>> + // 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@ffmpeg.org>https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>To
>> unsubscribe, visit link above, or email>ffmpeg-devel-requ...@ffmpeg.org
>> with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
