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 25F0145F7B for ; Thu, 20 Apr 2023 16:27:42 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A359F68BEB3; Thu, 20 Apr 2023 19:27:39 +0300 (EEST) Received: from mail-yw1-f178.google.com (mail-yw1-f178.google.com [209.85.128.178]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 2FA5468B9EB for ; Thu, 20 Apr 2023 19:27:33 +0300 (EEST) Received: by mail-yw1-f178.google.com with SMTP id 00721157ae682-5560fba3cacso7527b3.1 for ; Thu, 20 Apr 2023 09:27:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682008051; x=1684600051; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=rM9m1fCjIazCHFwkRMODO3C9RJiFcl8by45ImWnOUHU=; b=TnSJ2tCzQdZJ8oLku2W47uqL3SwdY30C5g/OXyEUC7YlY0WfYbMVDVeAaNguOJQ+Ua fhYTrGyMGIejaQagUjmZR++5Y4xs/jDPmL1l+deAo/94zszOJCk0p0bqG54cpV23wI2l OXP4c+cahATQscw4MuMP6lfwQbxFrLgv9jbM7h9woOQj9IVS0iUzIw7BHCa8KTmEYoVu e4s5xA+fG/LRBYUwsudt6MljpqiL4CoHeGDL6w/W/LdzH9ofWXVBTR+dVoMeoGnK3aA/ 1rsqgdezEHztlc9JrDLPHRTeR9EvbvqIrHBxlMy2DgfLA3HUskmXiPFBp7PxEyZcZRxK HL6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682008051; x=1684600051; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=rM9m1fCjIazCHFwkRMODO3C9RJiFcl8by45ImWnOUHU=; b=BROVo7gzlePo5iDpI+slJKTcl6gzkMpo17May15nzWW889ov6ygIawKcP6aLTQnmN7 yoNidsh3JMOUB4PvbapFzgG3dUWxYrACOG7HhS2nQMzlw3GE3a9ewOkI7xcwCbMosrMx MFj8foo/G8zrerOUopBChyzND2oeu2TdKyd7sa7Za6gJkitdzyv/S9iIHHTD9Sw477RF OojvPBQO3aFxVHGBmYY89JDDZ3PE9p2lBnvw9ivnZKR5j6KDdm+xUaIlPLZrsMUlolUL VNyWoo+0imIaLgqkLVBPhVYn+JZE68D18NXA24X+T9FIdE8Te9NoDME96rS5jngHoa6B +Evw== X-Gm-Message-State: AAQBX9dpdOsuKAzHJ4xNYWKlXEcmqlS2S3XMG9TG5jhdj6AV1ggc0RQx +R8QzG402m+j/mqZ+EgWzWeFs3ltv6E= X-Google-Smtp-Source: AKy350Zd4ws/gT+emqu+vo+3zHMi+AHeWH4AfKqV24VgDP13WORNtK9AyucHDsQxuK+niIk7XCGJog== X-Received: by 2002:a81:1710:0:b0:53c:70c5:45d2 with SMTP id 16-20020a811710000000b0053c70c545d2mr1219714ywx.0.1682008051646; Thu, 20 Apr 2023 09:27:31 -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 v131-20020a816189000000b00545a08184c9sm419640ywb.89.2023.04.20.09.27.31 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 20 Apr 2023 09:27:31 -0700 (PDT) Message-ID: <30517ac0-c63b-a615-3f8a-0d67922993ab@gmail.com> Date: Thu, 20 Apr 2023 12:27:30 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 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> <76d79ef5-b059-240c-7a95-8385d728b2e2@gmail.com> <20230420095026.GW275832@pb2> Content-Language: en-US-large From: Leo Izen In-Reply-To: <20230420095026.GW275832@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/20/23 05:50, Michael Niedermayer wrote: > On Wed, Apr 19, 2023 at 05:15:00PM -0400, Leo Izen wrote: >> 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. > > You might run into problems with user provided buffers if teh ordering is > entirely done at the end. because when decoding lets say red it should be in > the red plane. The user app might not expect its planes swapped > As far as I'm aware, I should be able to just rotate the AVFrame->data pointers and AVFrame->linesize values, as we don't promise these are ordered in the same order as the AVBufferRefs. _______________________________________________ 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".