From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 6699C45E1B for ; Mon, 12 Jun 2023 14:17:11 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 07EF868AF6B; Mon, 12 Jun 2023 17:17:08 +0300 (EEST) Received: from mail-oa1-f49.google.com (mail-oa1-f49.google.com [209.85.160.49]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1CA3368AFCE for ; Mon, 12 Jun 2023 17:17:02 +0300 (EEST) Received: by mail-oa1-f49.google.com with SMTP id 586e51a60fabf-1a6a2cb4659so310480fac.1 for ; Mon, 12 Jun 2023 07:17:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1686579420; x=1689171420; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=u6dSvFdnC26Q/F1/PdoGQCJOfZFzzEFik9k2jnErwZI=; b=p1SLlkjYX0wp7UUW7BTeeMzSkOcYM5rPbl6jjYsp7AmKwu74CVwYg6ZjBK38VPeoPb RVM8xCVr4D28kzCC/ica9p+yHYMV4LnCfhhAgKjE459SApi7KJGjQkwmeYRihqRgmHfm ReuLG0B/2kvK7qrQpGkE7cATJyqzm4xro7C/u9dwS6dTijEQubZMkKlX6Lh7Uh9q7hJj 8PPBW2WVudLhVQcozdvVTLyHiHMdARWn8QdV1k7qVSzN3I6RVsAvpXofG2u6bD0KG9SO 5zv1tUZKCqFlJ2Lg7Zw7Siwmmt3sDDC2c0BZ8v5o+qhiQi1eALr4UH3NftIwHxSiehdk 5WBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686579420; x=1689171420; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=u6dSvFdnC26Q/F1/PdoGQCJOfZFzzEFik9k2jnErwZI=; b=BnTVODXfvlimug4Sn3naJHCVIRCL7hczn+ZVu5bN9M1Prg0yJ+XZDgTwGxBthit2Ee WGDTj1eOTLRbYeDWYz0qWkLscU8NUm2goAQ82XAGJJ9dRB05ZyPJ9lezI7Pos9k04AiI FxgMZU2wnN3Ki4YndVuuW55UXeIj5tJo80cdjeRSX4Pa0Yw6e1KtqwrHnCSAE4ql8oGY jlkG+8IjLkgL8wt2ZegEiAZTK5ZKZBlHEgjIaHr53vL0qUJOT7FvE14PT44GDMKic5az gqKmgo02pRUAzpW9oMs1Q6aRyY9t0FXdIasjPe66698U5799BSSNhFOHJbzRjoyx6mUK ziCg== X-Gm-Message-State: AC+VfDzl9afbv0F9z8paVgSNZUuFlib7tqbS89ORUZv67UQv3kbB9Vwh 2lK9mvl1Jx7tgZw3Y/s763jQVJqq/H8= X-Google-Smtp-Source: ACHHUZ4d6LL2fmbyw1xea6XRIFO5RlDlvzUeTGENN3Kn+WuIaRcp0vD6dE21EMQ6y8ulJ/pMI1blvg== X-Received: by 2002:a05:6870:a70a:b0:196:5d42:b979 with SMTP id g10-20020a056870a70a00b001965d42b979mr6280034oam.3.1686579419395; Mon, 12 Jun 2023 07:16:59 -0700 (PDT) Received: from [192.168.0.12] (host197.190-225-105.telecom.net.ar. [190.225.105.197]) by smtp.gmail.com with ESMTPSA id eh11-20020a056870f58b00b0019f188355a8sm5884470oab.17.2023.06.12.07.16.58 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 12 Jun 2023 07:16:58 -0700 (PDT) Message-ID: <4e439dde-45d5-d00a-2428-250944757342@gmail.com> Date: Mon, 12 Jun 2023 11:16:57 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.2 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20230612122812.1153-1-d.kozinski@samsung.com> From: James Almer In-Reply-To: <20230612122812.1153-1-d.kozinski@samsung.com> Subject: Re: [FFmpeg-devel] [PATCH v24 1/9] avcodec/evc_parser: Added parser implementation for EVC format X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 6/12/2023 9:28 AM, Dawid Kozinski wrote: > +int ff_evc_parse_nal_units(EVCParserContext *ctx, const uint8_t *buf, int buf_size, void *logctx) > +{ > + const uint8_t *data = buf; > + int data_size = buf_size; > + int bytes_read = 0; > + int nalu_size = 0; > + > + while (data_size > 0) { > + > + // Buffer size is not enough for buffer to store NAL unit 4-bytes prefix (length) > + if (data_size < EVC_NALU_LENGTH_PREFIX_SIZE) > + return END_NOT_FOUND; END_NOT_FOUND is a return value from the packet merging functionality of AVParser, which you're not using anymore. Given the parser now expects complete NALUs, this should be AVERROR_INVALIDDATA. Also, ff_evc_parse_nal_units() seems to be used only by this parser, so why is it shared? It's ff_evc_parse_nal_unit() that is also used by the merge bsf. > + > + nalu_size = ff_evc_read_nal_unit_length(data, data_size, logctx); > + > + bytes_read += EVC_NALU_LENGTH_PREFIX_SIZE; > + > + data += EVC_NALU_LENGTH_PREFIX_SIZE; > + data_size -= EVC_NALU_LENGTH_PREFIX_SIZE; > + > + if (data_size < nalu_size) > + return END_NOT_FOUND; Same. > + > + if (ff_evc_parse_nal_unit(ctx, data, nalu_size, logctx) != 0) { > + av_log(logctx, AV_LOG_ERROR, "Parsing of NAL unit failed\n"); > + return AVERROR_INVALIDDATA; > + } > + > + data += nalu_size; > + data_size -= nalu_size; > + } > + return 0; > +} [...] > +/** > + * Parse NAL units of found picture and decode some basic information. > + * > + * @param s parser context. > + * @param avctx codec context. > + * @param buf buffer with field/frame data. > + * @param buf_size size of the buffer. > + */ > +static int parse_nal_units(AVCodecParserContext *s, AVCodecContext *avctx, const uint8_t *buf, int buf_size) > +{ > + EVCParserContext *p = s->priv_data; > + > + int res = ff_evc_parse_nal_units(p, buf, buf_size, avctx); > + if(!res) > + return res; ff_evc_parse_nal_units() returns 0 on success, so the check should be ret < 0. Otherwise all the code below will never be used. > + > + s->picture_structure = AV_PICTURE_STRUCTURE_FRAME; > + > + if(p->nalu_type == EVC_SPS_NUT) { Wont p->nalu_type here be the value of the last NAL parsed in the AU? If so, I'd expect it to be a picture and not a sequence header, so this code will rarely if ever be covered. Since ff_evc_parse_nal_units() doesn't need to be shared, it can reside locally here and you can pass it the AVCodecParserContext pointer, setting all these values as soon as they are found while parsing the respective NALUs. > + > + s->coded_width = p->coded_width; > + s->coded_height = p->coded_height; > + s->width = p->width; > + s->height = p->height; > + > + s->format = p->format; > + > + avctx->coded_width = s->coded_width; > + avctx->coded_height = s->coded_height; > + avctx->width = s->width; > + avctx->height = s->height; Remove these four. They are set generically by the AVParser code. > + > + avctx->gop_size = p->gop_size; > + avctx->delay = p->delay; > + avctx->profile = p->profile; > + > + avctx->framerate.den = p->framerate.den; > + avctx->framerate.num = p->framerate.num; > + > + } else if(p->nalu_type == EVC_NOIDR_NUT) { > + s->pict_type = p->pict_type; > + s->key_frame = p->key_frame; > + s->output_picture_number = p->output_picture_number; Why export these only for non IDR pictures? > + } > + > + return 0; > +} _______________________________________________ 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".