>On 08/12/2020 06:19, s...@jkqxz.net wrote:>>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>>> --->> Changelog | 
>1 +>> configure | 4 +>> doc/decoders.texi | 21 +++>> doc/general.texi | 8 ++>> 
>libavcodec/Makefile | 1 +>> libavcodec/allcodecs.c | 1 +>> 
>libavcodec/libuavs3d.c | 283 +++++++++++++++++++++++++++++++++++++++++>> 7 
>files changed, 319 insertions(+)>> create mode 100644 
>libavcodec/libuavs3d.c>>>> ...>> diff --git a/libavcodec/libuavs3d.c 
>b/libavcodec/libuavs3d.c>> new file mode 100644>> index 
>0000000000..c0d89cd1ce>> --- /dev/null>> +++ b/libavcodec/libuavs3d.c>> @@ 
>-0,0 +1,283 @@>> +/*>> + * RAW AVS3-P2/IEEE1857.10 video demuxer>> + * 
>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 "libavutil/avassert.h">> 
>+#include "libavutil/avutil.h">> +#include "libavutil/common.h">> +#include 
>"libavutil/imgutils.h">> +#include "libavutil/opt.h">> +#include "avcodec.h">> 
>+#include "internal.h">> +#include "uavs3d.h">> +>> +#define 
>UAVS3D_MAX_FRAME_THREADS 48>This restriction seems weird. Is it reflecting 
>some intrinsic property of the codec, or might it change in future?
The restriction will be removed in the next version.
>> +>> +static const int color_primaries_tab[10] = {>> + AVCOL_PRI_RESERVED0 , 
>> // 0>> + AVCOL_PRI_BT709 , // 1>> + AVCOL_PRI_UNSPECIFIED , // 2>> + 
>> AVCOL_PRI_RESERVED , // 3>> + AVCOL_PRI_BT470M , // 4>> + AVCOL_PRI_BT470BG 
>> , // 5>> + AVCOL_PRI_SMPTE170M , // 6>> + AVCOL_PRI_SMPTE240M , // 7>> + 
>> AVCOL_PRI_FILM , // 8>> + AVCOL_PRI_BT2020 // 9>> +};>> +>> +static const 
>> int color_transfer_tab[15] = {>> + AVCOL_TRC_RESERVED0 , // 0>> + 
>> AVCOL_TRC_BT709 , // 1>> + AVCOL_TRC_UNSPECIFIED , // 2>> + 
>> AVCOL_TRC_RESERVED , // 3>> + AVCOL_TRC_GAMMA22 , // 4>> + AVCOL_TRC_GAMMA28 
>> , // 5>> + AVCOL_TRC_SMPTE170M , // 6>> + AVCOL_TRC_SMPTE240M , // 7>> + 
>> AVCOL_TRC_LINEAR , // 8>> + AVCOL_TRC_LOG , // 9>> + AVCOL_TRC_LOG_SQRT , // 
>> 10>> + AVCOL_TRC_BT2020_12 , // 11>> + AVCOL_TRC_SMPTE2084 , // 12>> + 
>> AVCOL_TRC_UNSPECIFIED , // 13>> + AVCOL_TRC_ARIB_STD_B67 // 14>> +};>> +>> 
>> +static const int color_matrix_tab[12] = {>> + AVCOL_SPC_RESERVED , // 0>> + 
>> AVCOL_SPC_BT709 , // 1>> + AVCOL_SPC_UNSPECIFIED , // 2>> + 
>> AVCOL_SPC_RESERVED , // 3>> + AVCOL_SPC_FCC , // 4>> + AVCOL_SPC_BT470BG , 
>> // 5>> + AVCOL_SPC_SMPTE170M , // 6>> + AVCOL_SPC_SMPTE240M , // 7>> + 
>> AVCOL_SPC_BT2020_NCL , // 8>> + AVCOL_SPC_BT2020_CL , // 9>> + 
>> AVCOL_SPC_UNSPECIFIED , // 10>> + AVCOL_SPC_UNSPECIFIED // 11>> +};>These 
>> tables aren't used anywhere.
These informations are located in the sequence extension header, and the tables 
are ID maps between FFMPEG ID ans AVS3 ID.We will add extracting these 
informations in the next version.
>> +>> +static const enum AVPictureType IMGTYPE[8] = {>This name is doesn't 
>> conform to the usual naming convention. uavs3_frame_types[], maybe?>> + 
>> AV_PICTURE_TYPE_NONE,>> + AV_PICTURE_TYPE_I,>> + AV_PICTURE_TYPE_P,>> + 
>> AV_PICTURE_TYPE_B>> +};>What do the other four values in the table mean?
Sorry, it is a mistake, and will be fixed.
>> +>> +typedef struct UAVS3DContext {>> + AVCodecContext *avctx;>> + void 
>> *dec_handle;>> + int frame_threads;>> + int got_seqhdr;>> + uavs3d_io_frm_t 
>> dec_frame;>> +} UAVS3DContext;>> +>> +>> +static int 
>> uavs3d_find_next_start_code(const unsigned char *bs_data, int bs_len, int 
>> *left)>> +{>> + const unsigned char *data_ptr = bs_data + 4;>> + int count = 
>> bs_len - 4;>> +>> + while (count >= 4 &&>> + ((*(unsigned int *)data_ptr) != 
>> 0xB6010000) && /* P/B picture */>This pointer cast will be undefined 
>> behaviour, because the data need not be aligned. Even if it were valid, it's 
>> still going to give the wrong answer on a machine with different endianness 
>> to the one you tested on. Use AV_RB32() or AV_RL32().
Thanks, we will use AV_RL32() instead of it.
>> + ((*(unsigned int *)data_ptr) != 0xB3010000) && /* I picture */>> + 
>> ((*(unsigned int *)data_ptr) != 0xB0010000) && /* sequence header */>> + 
>> ((*(unsigned int *)data_ptr) != 0x00010000) && /* first slice */>> + 
>> ((*(unsigned int *)data_ptr) != 0xB1010000)) { /* sequence end */>Make 
>> symbolic constants for the magic numbers here.
Thanks, these will be fixed.
>> + data_ptr++;>> + count--;>> + }>> +>> + if (count >= 4) {>> + *left = 
>> count;>> + return 1;>> + }>> +>> + return 0;>> +}>> +>> +static void 
>> uavs3d_output_callback(uavs3d_io_frm_t *dec_frame) {>> + uavs3d_io_frm_t 
>> frm_out;>> + AVFrame *frm = (AVFrame *)dec_frame->priv;>> + int i;>> +>> + 
>> if (!frm) {>> + return;>> + }>> +>> + frm->pts = dec_frame->pts;>> + 
>> frm->pkt_dts = dec_frame->dts;>> + frm->pict_type = 
>> IMGTYPE[dec_frame->type];>Is overflow possible?
It will be fixed.
>> + frm->key_frame = (frm->pict_type == AV_PICTURE_TYPE_I);>> +>> + for (i = 
>> 0; i < 3; i++) {>> + frm_out.width [i] = dec_frame->width[i];>> + 
>> frm_out.height[i] = dec_frame->height[i];>> + frm_out.stride[i] = 
>> frm->linesize[i];>> + frm_out.buffer[i] = frm->data[i];>> + }>> +>> + 
>> uavs3d_img_cpy_cvt(&frm_out, dec_frame, dec_frame->bit_depth);>Is this copy 
>> really required? Decoders will normally allow you to reference the frame 
>> they hold internally.>Can the call ever fail?
Sorry, the decoder will reuse the buffer of dec_frame in the next decoding, so 
the copy is necessory.If the call "uavs3d_img_cpy_cvt" fail, an error image 
will be display. The error code is useless, and it not easy for error check 
with SIMD instructions for various CPUs.We will check if the buffer is vaild in 
the next version. If not, set the got_pic flag with false.
>> +}>> +>> +static av_cold int libuavs3d_init(AVCodecContext *avctx)>> +{>> + 
>> UAVS3DContext *h = avctx->priv_data;>> + uavs3d_cfg_t cdsc;>> +>> + 
>> cdsc.frm_threads = FFMIN(h->frame_threads > 0 ? h->frame_threads : 
>> av_cpu_count(), UAVS3D_MAX_FRAME_THREADS);>> + cdsc.check_md5 = 0;>> + 
>> h->dec_handle = uavs3d_create(&cdsc, uavs3d_output_callback, NULL);>Can this 
>> call fail? (For example, if out of memory.)
Thanks, it will be fixed.
>> + h->got_seqhdr = 0;>> +>> + return 0;>> +}>> +>> +static av_cold int 
>> libuavs3d_end(AVCodecContext *avctx)>> +{>> + UAVS3DContext *h = 
>> avctx->priv_data;>> +>> + if (h->dec_handle) {>> + 
>> uavs3d_flush(h->dec_handle, NULL);>> + uavs3d_delete(h->dec_handle);>> + 
>> h->dec_handle = NULL;>> + }>> + h->got_seqhdr = 0;>> +>> + return 0;>> +}>> 
>> +>> +static void libuavs3d_flush(AVCodecContext * avctx)>> +{>> + 
>> UAVS3DContext *h = avctx->priv_data;>> + uavs3d_cfg_t cdsc;>> + 
>> cdsc.frm_threads = FFMIN(h->frame_threads > 0 ? h->frame_threads : 
>> av_cpu_count(), UAVS3D_MAX_FRAME_THREADS);>> + cdsc.check_md5 = 0;>> +>> + 
>> if (h->dec_handle) {>> + uavs3d_flush(h->dec_handle, NULL);>> + 
>> uavs3d_delete(h->dec_handle);>> + }>> +>> + h->dec_handle = 
>> uavs3d_create(&cdsc, uavs3d_output_callback, NULL);>> + h->got_seqhdr = 0;>> 
>> +}>> +>> +static int libuavs3d_decode_frame(AVCodecContext *avctx, void 
>> *data, int *got_frame, AVPacket *avpkt)>> +{>> + UAVS3DContext *h = 
>> avctx->priv_data;>> + const uint8_t *buf = avpkt->data;>> + int buf_size = 
>> avpkt->size;>> + const uint8_t *buf_end;>> + const uint8_t *buf_ptr;>> + 
>> AVFrame *frm = (AVFrame*)data;>No need for explicit casts from void*.
Thanks, it will be fixed.
>> + int left_bytes;>> + int ret, finish = 0;>> +>> + *got_frame = 0;>> + 
>> frm->pts = -1;>> + frm->pict_type = AV_PICTURE_TYPE_NONE;>> +>> + if 
>> (h->got_seqhdr) {>> + if (!frm->data[0] && (ret = ff_get_buffer(avctx, frm, 
>> 0)) < 0) {>> + return ret;>> + }>> + h->dec_frame.priv = data; // 
>> AVFrame>Always allocating here if you have sequence header looks suspicious. 
>> What if you don't get any frame output?
Thanks, we will free the buffer in the next version if no frame is outputed. We 
don't know whether one frame will be got before decoding, so it is a simple but 
effective implementation to allocate the buffer once the frame size and pix_fmt 
are received. 
>> + }>> +>> + if (!buf_size) {>> + do {>> + ret = uavs3d_flush(h->dec_handle, 
>> &h->dec_frame);>> + } while (ret > 0 && !h->dec_frame.got_pic);>> + } else 
>> {>> + buf_ptr = buf;>> + buf_end = buf + buf_size;>> +>> + while (!finish) 
>> {>> + int bs_len;>> + uavs3d_io_frm_t *frm_dec = &h->dec_frame;>> +>> + if 
>> (uavs3d_find_next_start_code(buf_ptr, buf_end - buf_ptr, &left_bytes)) {>> + 
>> bs_len = buf_end - buf_ptr - left_bytes;>> + } else {>> + bs_len = buf_end - 
>> buf_ptr;>> + finish = 1;>> + }>> + frm_dec->bs = (unsigned char *)buf_ptr;>> 
>> + frm_dec->bs_len = bs_len;>> + frm_dec->pts = avpkt->pts;>> + frm_dec->dts 
>> = avpkt->dts;>> + uavs3d_decode(h->dec_handle, frm_dec);>Does this call 
>> return any sort of error?>More generally, what happens if there is an error 
>> in decoding? (For example, because the stream is corrupted.)
Thanks, errors will be checked.
>> + buf_ptr += bs_len;>> +>> + if (frm_dec->nal_type == NAL_SEQ_HEADER) {>> + 
>> 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 };>> + avctx->framerate.num = 
>> avs3_fps_num[frm_dec->seqhdr->frame_rate_code];>> + avctx->framerate.den = 
>> avs3_fps_den[frm_dec->seqhdr->frame_rate_code];>Same comment as in 2/4 about 
>> ff_mpeg12_frame_rate_tab[].
It will be fixed.
>> + avctx->has_b_frames = 1;>Always?
In AVS3, no flag shows whether B frames exist, we will choose the low delay 
flag for it. 
>> + avctx->pix_fmt = frm_dec->seqhdr->bit_depth_internal == 8 ? 
>> AV_PIX_FMT_YUV420P : AV_PIX_FMT_YUV420P10LE;>> + ff_set_dimensions(avctx, 
>> frm_dec->seqhdr->horizontal_size, frm_dec->seqhdr->vertical_size);>> + 
>> h->got_seqhdr = 1;>> + }>> + if (frm_dec->got_pic) {>> + break;>> + }>> + 
>> }>> + }>> +>> + *got_frame = h->dec_frame.got_pic;>> +>> + return buf_ptr - 
>> buf;>> +}>> +>> +#define UAVS3D_OFFSET(x) offsetof(UAVS3DContext, x)>> 
>> +#define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM>> +static 
>> const AVOption options[] = {>> + { "frame_threads", "number of frame-level 
>> threads ", UAVS3D_OFFSET(frame_threads), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 
>> UAVS3D_MAX_FRAME_THREADS, VE },>> + { NULL }>> +};>> +static const AVClass 
>> libuavs3d_class = {>> + .class_name = "libuavs3d_class",>> + .item_name = 
>> av_default_item_name,>> + .option = options,>> + .version = 
>> LIBAVUTIL_VERSION_INT,>> +};>> +>> +AVCodec ff_libuavs3d_decoder = {>> + 
>> .name = "libuavs3d",>> + .long_name = NULL_IF_CONFIG_SMALL("uavs3d 
>> AVS3-P2/IEEE1857.10 decoder"),>> + .type = AVMEDIA_TYPE_VIDEO,>> + .id = 
>> AV_CODEC_ID_AVS3,>> + .priv_data_size = sizeof(UAVS3DContext),>> + 
>> .priv_class = &libuavs3d_class,>> + .init = libuavs3d_init,>> + .close = 
>> libuavs3d_end,>> + .decode = libuavs3d_decode_frame,>> + .capabilities = 
>> AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,>> + .flush = libuavs3d_flush,>> + 
>> .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV420P, 
>> AV_PIX_FMT_YUV420P10LE, AV_PIX_FMT_NONE },>> +>> + .wrapper_name = 
>> "libuavs3d",>> +};>>>- 
>> 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".

Reply via email to