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 3BF4140BCB for ; Sat, 5 Feb 2022 04:26:40 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7016968B30D; Sat, 5 Feb 2022 06:26:38 +0200 (EET) Received: from mail-qk1-f171.google.com (mail-qk1-f171.google.com [209.85.222.171]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A399E68B0E8 for ; Sat, 5 Feb 2022 06:26:31 +0200 (EET) Received: by mail-qk1-f171.google.com with SMTP id j24so6405252qkk.10 for ; Fri, 04 Feb 2022 20:26:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :references:from:in-reply-to; bh=Ht+losd14cES7slMLecGIjHKkgyLpJlAQuwNkIWMIuQ=; b=CIqM3tqaODti8IoSeCc1EryUxNZq+WwIzbyKbQe6Xjnf3HU+iOccsHXVsWWm86CLaM MV3gB8SjYLh43uE3Ffn1UG7QeXDCI7/50m5VRI1MseqkfhUH766wgk5TyFILUN8m8jd5 oYnPlcYAAe2vPHxwbqw4k9RFU6Vc2WbsE/NILM5vF3GO7Bp5SD5K0w1midK5Hz2bTl4B iG3ppkFTcv7WVjt0WN912fY7T7QD0vXrWrrdT5O12tfZL79VbuVdRgWoQtLQKdkjyRyh eWLbKwTnxgtPHInini9o2p/3ldylMLimIx9M9aej1BHydFByR4oxd4AjezzOUcAZVWkR 6eYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to; bh=Ht+losd14cES7slMLecGIjHKkgyLpJlAQuwNkIWMIuQ=; b=gGrl/AbAxRhF0t6k9Pu9yymsPVdT398DSkLR7RdHKzseiPwYAwr2XT/l9TXXQWMgZu rS3i9pFXaouL4Ns9xlBx2xI/QpOiLRwGS8Pj5hFFPX4+spr/gnCe3fBWwAS+XjMLGHEg 4GAdhjK1KMEOvrR4wH+twyeSagK6YmeCg88IIvA7CcEdmr7Jv4paQ82sScrTXTjRO8Zi +ZZZS7fLMJN18qtiBfj+ZVhqGlwkU+OK9IaH6AXFKcKzBctAstZYmkShMAX/7N9dpqQ2 FB7xZ24UpRzG7zD3imtthAaMwAUfFxa0LLSUjA1Ot7IGIDnf/2s1Y7tiQ8KlVS9TJKCx vfHQ== X-Gm-Message-State: AOAM531PvYS4fvx/y6FJD1zRFMtObgLGz98zJ8HIQGgGIEhAG5P5jH+9 5aUHX2AnxSIAcWiyoanfp3eSs/aG8YmFKA== X-Google-Smtp-Source: ABdhPJxSNnNR9iMnWzPiFh5/zUS5zV58BnrQhmUY6htM+qIJogW1XBfdQCmV+f+9BphTBjMMr/czZw== X-Received: by 2002:a37:a808:: with SMTP id r8mr1202814qke.665.1644035190024; Fri, 04 Feb 2022 20:26:30 -0800 (PST) Received: from [192.168.1.64] ([151.200.235.219]) by smtp.gmail.com with ESMTPSA id m3sm2244910qkp.100.2022.02.04.20.26.29 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 04 Feb 2022 20:26:29 -0800 (PST) Message-ID: <9aef3a61-9f9e-d6b1-aff2-56d68fe2ae93@gmail.com> Date: Fri, 4 Feb 2022 23:26:28 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220201212056.29712-1-scott.the.elm@gmail.com> <20220203184450.5491-1-scott.the.elm@gmail.com> <20220203184450.5491-14-scott.the.elm@gmail.com> From: Scott Theisen In-Reply-To: X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [FFmpeg-devel] [PATCH v2 13/13] cbs_mpeg2.c: improve readability of start code search (part 3) 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 2/4/22 22:54, Andreas Rheinhardt wrote: > I disagree that this is the true loop condition that just needs to be > revealed; in fact, this is basically the loop condition from before > fd93d5efe64206d5f1bce8c702602353444c0c1a, just with an added block to > deal with size one units at the end. I considered this and chose the > current one because it leads to less code duplication for this special case. > Anyway, now that I have taken another look at this and I finally found > the true loop condition: Is there a unit that we have not added to the > fragment yet? This is easily implementable if one always resets the > start code, so that there being an outstanding unit is equivalent to the > start code being valid. Looking at your patch series again, I agree your changes to cbs_mpeg2.c are clearer. However, I think my changes from patch 11 are a further helpful clarification (ignoring the index and loop changes (since you already did that) and the "redundant" comment): @@ -144,23 +144,24 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx, CodedBitstreamFragment *frag, int header) { - const uint8_t *start, *end; + const uint8_t *start = frag->data, *end; + const uint8_t * const buf_end = frag->data + frag->data_size; CodedBitstreamUnitType unit_type; uint32_t start_code = -1; size_t unit_size; - int err, i = 0, final = 0; + int err, final = 0; + int i = -1; // offset for pre-increment - start = avpriv_find_start_code(frag->data, frag->data + frag->data_size, - &start_code, 1); + start = avpriv_find_start_code(start, buf_end, &start_code, 1); if (!avpriv_start_code_is_valid(start_code)) { // No start code found. return AVERROR_INVALIDDATA; } - while (!final) { + do { unit_type = start_code & 0xff; - if (start == frag->data + frag->data_size) { + if (start == buf_end) { // The last four bytes form a start code which constitutes // a unit of its own. In this situation avpriv_find_start_code // won't modify start_code at all so modify start_code so that @@ -168,10 +169,9 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx, start_code = 0; } - end = avpriv_find_start_code(start--, frag->data + frag->data_size, - &start_code, 0); - - // start points to the byte containing the start_code_identifier + end = avpriv_find_start_code(start, buf_end, &start_code, 0); + start--; + // decrement so start points to the byte containing the start_code_identifier // (may be the last byte of fragment->data); end points to the byte // following the byte containing the start code identifier (or to // the end of fragment->data). Should I submit a v3 patch series which only includes patches 1-9? (That is only the avpriv_find_start_code() changes, since 10-13 were cbs_mpeg2.c and separate but related.) Regards, Scott _______________________________________________ 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".