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 818F145F75 for ; Wed, 19 Apr 2023 21:15:12 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0497068BEB6; Thu, 20 Apr 2023 00:15:10 +0300 (EEST) Received: from mail-yb1-f178.google.com (mail-yb1-f178.google.com [209.85.219.178]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9E60F68BB66 for ; Thu, 20 Apr 2023 00:15:03 +0300 (EEST) Received: by mail-yb1-f178.google.com with SMTP id 3f1490d57ef6-b8f48cd693eso72321276.0 for ; Wed, 19 Apr 2023 14:15:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1681938902; x=1684530902; 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=z5lYRXvnzhF9OL8FirZ/a0s8W8G4iv2ZH2N4InyjIeE=; b=suTvc7KSK5gURUQGod8mlbdmS1Ot8i4JD8RTGleOq5HPwocvZvpeJQDZ4fDhPNtvvU 1vOFjX2kLteGZllDu/NnFK6O9QrQHeJM0WIZb7QejHtqpyyJ6HbiMW0OnqepHzohx+N8 UCYrboRCJyCypWqSijkdmqn1GU3ljcB0RmpjrR1MoZ/ireXGHdYwWcUR7v8ZrsI8ZmGX bMEup8KvidJ52wMW52cq/k3tpWd2oKp85S3DHBXx/7ScXEDhgORTMit29Zd21S0rYc2k ID2AhK8ujj0Ps2wJuEW7RgBA7qKy6JT1n7eVgihGV4vkjc6glTgLt8Om7XLY4N+Mf1zE 4q3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681938902; x=1684530902; 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=z5lYRXvnzhF9OL8FirZ/a0s8W8G4iv2ZH2N4InyjIeE=; b=AvbSL4MmoNpv+LhbCTZrOvGNwloTcEYkQlnb5YmymqGG+jVZnJk7E0zTPNxG3hDXaV GfLHdFmfDVmWj/VNDuqenx+qqFrvF34jrH4j3+9RWLlVv5+Snqxlq03ZudoHb26VFjqI UFHJoQkUDqpveLgqtWnKXjL3D+SRfoRFLVxzs76d811FYLm5ifeC6si7zInbMIJ32sTY GeUwgJiJmggRoiTGGuh00xCsbf6ClPHzMmEcmGrdcwNKVINQd2MDZ/SHyiHwxk0MPE1U oy2DXsbv2vdKHiPOsdXiPq2ipGtDjWU3UzYn8yC0PN1i7jWttm0aSr30OJQFvLNiRUP/ 5o8w== X-Gm-Message-State: AAQBX9d4K1DWy3Yhp4f0rhyjr3zwBP+dr76D8MbdwCbPdgoG+hy16/Px WKFP/ka8BU6JYBc7O0Z6tUbqqv6k0fM= X-Google-Smtp-Source: AKy350buEDG1e3itL5CA0j1ZcOk7ULA5NU5yqvQN605MrY6MDwhfhWuI/I2vfnCIDO56sc1XIoPh1A== X-Received: by 2002:a81:1c87:0:b0:54f:8562:e36 with SMTP id c129-20020a811c87000000b0054f85620e36mr17351714ywc.1.1681938901809; Wed, 19 Apr 2023 14:15:01 -0700 (PDT) Received: from [192.168.1.35] (c-98-224-219-15.hsd1.mi.comcast.net. [98.224.219.15]) by smtp.gmail.com with ESMTPSA id 191-20020a8106c8000000b00545a081849esm4743336ywg.46.2023.04.19.14.15.01 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Apr 2023 14:15:01 -0700 (PDT) Message-ID: <76d79ef5-b059-240c-7a95-8385d728b2e2@gmail.com> Date: Wed, 19 Apr 2023 17:15:00 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Content-Language: en-US-large To: ffmpeg-devel@ffmpeg.org References: <20230419181126.38662-1-leo.izen@gmail.com> <20230419181126.38662-2-leo.izen@gmail.com> <20230419185838.GS275832@pb2> <1e1d5c0d-7b06-3215-706c-edfc65a72801@gmail.com> <20230419203732.GT275832@pb2> From: Leo Izen In-Reply-To: <20230419203732.GT275832@pb2> Subject: Re: [FFmpeg-devel] [PATCH v3 1/3] avcodec/mjpegdec: fix non-subsampled RGB JPEGs 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 4/19/23 16:37, Michael Niedermayer wrote: > On Wed, Apr 19, 2023 at 03:23:41PM -0400, Leo Izen wrote: >> On 4/19/23 14:58, Michael Niedermayer wrote: >>> On Wed, Apr 19, 2023 at 02:11:24PM -0400, Leo Izen wrote: >>>> The change introduced in b18a9c29713abc3a1b081de3f320ab53a47120c6 >>>> created a regression for non-subsampled progressive RGB jpegs. This >>>> should fix that. >>>> --- >>>> libavcodec/mjpegdec.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c >>>> index 01537d4774..1e3ddb72fb 100644 >>>> --- a/libavcodec/mjpegdec.c >>>> +++ b/libavcodec/mjpegdec.c >>>> @@ -1698,7 +1698,8 @@ int ff_mjpeg_decode_sos(MJpegDecodeContext *s, const uint8_t *mb_bitmask, >>>> s->h_scount[i] = s->h_count[index]; >>>> s->v_scount[i] = s->v_count[index]; >>>> - if(nb_components == 3 && s->nb_components == 3 && s->avctx->pix_fmt == AV_PIX_FMT_GBRP) >>>> + if((nb_components == 3 || nb_components == 1) && s->nb_components == 3 >>>> + && s->avctx->pix_fmt == AV_PIX_FMT_GBRP && !s->progressive) >>>> index = (index+2)%3; >>> >>> Why is progressive/!progressive special cased in all the new RGB code ? >>> >> >> With progressive, I decode RGB in RGB-order, and then pivot it into >> GBR-order, whereas baseline is just decoded directly into GBR-order. If you >> decode progressive directly in GBR-order the buffers will be the wrong size >> and it will overrun the subsampled buffer when filling it with a >> non-subsampled one. See the allocation block on line 766 of mjpegdec.c. This >> depends on h_count and v_count, which cannot be changed or pivoted as if you >> do so, progressive JPEGs will fail to decode at all (invalid VLC entries, >> etc.) >> >> Ideally, you'd just alloc them the right size, but s->component_index[i] >> won't refer to the right index for many progressive files, depending on >> whether the SOS marker has 1 or 3 components. If you have SOS markers with >> one component it will not properly pivot the colors. >> >> Initially, I didn't have the checks and just always decoded in RGB order and >> then pivoted, but that broke some baseline files like the ones in Trac >> #4045. I used some casework so I could handle all files I tested with this. >> >> If anyone has any suggestions on how to make the casework more elegant I'm >> all ears but this is the solution I found to work with every sample I >> tested. > > First i would document which array is in PIXFMT vs. JPEG order > when anything is in 2 different orders at different points or for > different cases thats probably not a good idea. > But even if such bad cases exist, it should be documented > > progressive is complicated because one could argue that it needs > to be possible to both add pieces into the image and also to > make these pieces immedeatly available to the user so some > application could present to the user the image as it is > "progressing". Ok we maybe dont care for that feature but its > still not a bad way to look at the problem. > I presume all jpeg streams can be decoded without too much problems > if everything is in jpeg order. > at the same time to present it we need planes to be scaled and > ordered into a standard RGB/GBR/YUV form. > I think these 2 worlds JPEG vs presentation should be more clearly > seperated, > > am i seeing the issue correctly or am i missing the problems here ? > > thx I could try to see if I can decode *every* image in JPEG order and then pivot the planes from RGB to GBR order at the end, but it might take me a bit more time to figure it out. I'll take a look at it this week. It would be a more elegant solution and wouldn't require us to document which planes go where in which places of the code. - Leo Izen _______________________________________________ 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".