Re: [FFmpeg-devel] [PATCH v19 02/10] avcodec/evc_parser: Added parser implementation for EVC format

["Dawid Kozinski/Multimedia \(PLT\) /SRPOL/Staff Engineer/Samsung Electronics" <d.kozinski@samsung.com>]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James
> Almer
> Sent: poniedziałek, 17 kwietnia 2023 00:17
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v19 02/10] avcodec/evc_parser: Added
> parser implementation for EVC format
> 
> On 4/5/2023 4:01 AM, Dawid Kozinski wrote:
> > +static int parse_nal_unit(AVCodecParserContext *s, const uint8_t *buf,
> > +                          int buf_size, AVCodecContext *avctx) {
> > +    EVCParserContext *ev = s->priv_data;
> > +    int nalu_type, nalu_size;
> > +    int tid;
> > +    const uint8_t *data = buf;
> > +    int data_size = buf_size;
> > +
> > +    s->picture_structure = AV_PICTURE_STRUCTURE_FRAME;
> > +    s->key_frame = -1;
> > +
> > +    nalu_size = buf_size;
> > +    if (nalu_size <= 0) {
> > +        av_log(avctx, AV_LOG_ERROR, "Invalid NAL unit size: (%d)\n",
nalu_size);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    // @see ISO_IEC_23094-1_2020, 7.4.2.2 NAL unit header semantic
(Table 4
> - NAL unit type codes and NAL unit type classes)
> > +    // @see enum EVCNALUnitType in evc.h
> > +    nalu_type = get_nalu_type(data, data_size, avctx);
> > +    if (nalu_type < EVC_NOIDR_NUT || nalu_type > EVC_UNSPEC_NUT62) {
> > +        av_log(avctx, AV_LOG_ERROR, "Invalid NAL unit type: (%d)\n",
> nalu_type);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    ev->nalu_type = nalu_type;
> > +
> > +    tid = get_temporal_id(data, data_size, avctx);
> > +    if (tid < 0) {
> > +        av_log(avctx, AV_LOG_ERROR, "Invalid temporial id: (%d)\n",
tid);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    ev->nuh_temporal_id = tid;
> > +
> > +    if (data_size < nalu_size) {
> > +        av_log(avctx, AV_LOG_ERROR, "NAL unit does not fit in the data
> buffer\n");
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    data += EVC_NALU_HEADER_SIZE;
> > +    data_size -= EVC_NALU_HEADER_SIZE;
> > +
> > +    switch(nalu_type) {
> > +    case EVC_SPS_NUT: {
> > +        EVCParserSPS *sps;
> > +        int SubGopLength;
> > +
> > +        sps = parse_sps(data, nalu_size, ev);
> > +        if (!sps) {
> > +            av_log(avctx, AV_LOG_ERROR, "SPS parsing error\n");
> > +            return AVERROR_INVALIDDATA;
> > +        }
> > +
> > +        s->coded_width         = sps->pic_width_in_luma_samples;
> > +        s->coded_height        = sps->pic_height_in_luma_samples;
> > +        s->width               = sps->pic_width_in_luma_samples  - sps-
> >picture_crop_left_offset - sps->picture_crop_right_offset;
> > +        s->height              = sps->pic_height_in_luma_samples - sps-
> >picture_crop_top_offset  - sps->picture_crop_bottom_offset;
> > +
> > +        SubGopLength = (int)pow(2.0, sps->log2_sub_gop_length);
> > +        avctx->gop_size = SubGopLength;
> > +
> > +        avctx->delay = (sps->sps_max_dec_pic_buffering_minus1) ?
> > + sps->sps_max_dec_pic_buffering_minus1 - 1 : SubGopLength +
> > + sps->max_num_tid0_ref_pics - 1;
> > +
> > +        if (sps->profile_idc == 1) avctx->profile =
FF_PROFILE_EVC_MAIN;
> > +        else avctx->profile = FF_PROFILE_EVC_BASELINE;
> > +
> > +        if (sps->vui_parameters_present_flag) {
> > +            if (sps->vui_parameters.timing_info_present_flag) {
> > +                int64_t num = sps->vui_parameters.num_units_in_tick;
> > +                int64_t den = sps->vui_parameters.time_scale;
> > +                if (num != 0 && den != 0)
> > +                    av_reduce(&avctx->framerate.den,
&avctx->framerate.num, num,
> den, 1 << 30);
> > +            }
> > +        }
> > +
> > +        switch (sps->chroma_format_idc) {
> > +        case 0: /* YCBCR400_10LE */
> > +            av_log(avctx, AV_LOG_ERROR, "YCBCR400_10LE: Not supported
> chroma format\n");
> > +            s->format = AV_PIX_FMT_GRAY10LE;
> > +            return -1;
> 
> This is a parser, and it's independent from any given decoder
implementation.
> You should set it to all supported values and not abort on any.
> 


Thank you for your review.
I fixed mentioned piece of code and pushed a new patch set.
Let me know if smth else should be fixed.
I'm looking forward to your feedback.

> > +        case 1: /* YCBCR420_10LE */
> > +            s->format = AV_PIX_FMT_YUV420P10LE;
> > +            break;
> > +        case 2: /* YCBCR422_10LE */
> > +            av_log(avctx, AV_LOG_ERROR, "YCBCR422_10LE: Not supported
> chroma format\n");
> > +            s->format = AV_PIX_FMT_YUV422P10LE;
> > +            return -1;
> > +        case 3: /* YCBCR444_10LE */
> > +            av_log(avctx, AV_LOG_ERROR, "YCBCR444_10LE: Not supported
> chroma format\n");
> > +            s->format = AV_PIX_FMT_YUV444P10LE;
> > +            return -1;
> > +        default:
> > +            s->format = AV_PIX_FMT_NONE;
> > +            av_log(avctx, AV_LOG_ERROR, "Unknown supported chroma
> format\n");
> > +            return -1;
> > +        }
> > +        break;
> > +    }
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://protect2.fireeye.com/v1/url?k=926dc5cc-f3102db0-926c4e83-
> 74fe485cc33c-e1a37e8c9d3d64a9&q=1&e=c96a9555-c0b8-4eb4-bca2-
> ed6ef3868c42&u=https%3A%2F%2Fffmpeg.org%2Fmailman%2Flistinfo%2Fffmp
> eg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".