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 6111340D9E for ; Sun, 5 Feb 2023 04:57:35 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DEFE268BEB2; Sun, 5 Feb 2023 06:57:32 +0200 (EET) Received: from mail-io1-f48.google.com (mail-io1-f48.google.com [209.85.166.48]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D916868BCD7 for ; Sun, 5 Feb 2023 06:57:25 +0200 (EET) Received: by mail-io1-f48.google.com with SMTP id y7so3415693iob.6 for ; Sat, 04 Feb 2023 20:57:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=aY2yds524+Du6dONfXrFzy37T2eu2B3P9tVdy+ZE/7M=; b=EPVEjzy0Z6AYMQQGjVyuhiwy16ja6F0wVX5kLeH8qcpJ33O2kLatLucbBqMY/HM8uk stIzKx58JGrnCiTM5d5nVihsJV/Ik4jhSzSviARDvXEG+gOjTko3ZFr2pacMlNClPvk/ z0rLPSqZKLuqmY3xCwcuPprsWSP/8muH0CqVbCsOBvlvpMPGYv/2+l8wQCdtqjdZ7BBb 89WUlQ7BlojGR+Dp94ewuyc70RwQBFjnGAHJBEiIRtrktK/QbM9WeYuZZpfIBQ5fP/Zg 9DPyGlfzsLemgFyfpz89ZQBXCNA2jNBU8HPte818Gtw5ntP4RHFSGfsEfJtFrUrLCFAL ldcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=aY2yds524+Du6dONfXrFzy37T2eu2B3P9tVdy+ZE/7M=; b=tQAU4VAjuPfb8GSg9z81x9MoZcCBjiHp1JlmUKbPl8zGQsKd50q+/ebGXA6uMVvIky GLexeVxfWfj3CbzeRxkXBL0ZKamRyHsw7n4ZLCjZvLHz48l4REZiH3HqiY+SViCJrbvg unjUAIvXtnkV/WqLo1oaRhKxJHXCcbkaoLdn2sRC88+Sk+Rz2F8+e/7kkT3SbKQSyzF4 Y0UeFZexP4HE8RCsDvTSzCMK9SGcz7UuBkmv4HOMZ9Iukz63IaelA0ykVp1w4pDFR9EB t/ayZI8/AQVlhJB1WdOt27+9Uec0Hw7hP+/RhiGW7iD/28ryal7szmKRm7wvzW2kok+A uuaA== X-Gm-Message-State: AO0yUKVapt6T68guT9/KihLrYwAOHPAsEIeiYoyE1n5WxlZvouOE9KeZ pcHYXn+SRPzH8B+xwxnaypPF/0XQMuNlKw== X-Google-Smtp-Source: AK7set9vNQIhnL45UOEWyoK+Gjre2YlIy8lSexhWI5vixgdlnVxWexCCVxnZ3lkKZLCODDWaS0N0QQ== X-Received: by 2002:a5e:8307:0:b0:707:6808:45c0 with SMTP id x7-20020a5e8307000000b00707680845c0mr11111913iom.1.1675573044139; Sat, 04 Feb 2023 20:57:24 -0800 (PST) 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 u15-20020a02b1cf000000b003a4e6b1e064sm2260964jah.90.2023.02.04.20.57.23 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 04 Feb 2023 20:57:23 -0800 (PST) Message-ID: <7c808e9c-36a5-ce7f-0dea-c0548e61b3e6@gmail.com> Date: Sat, 4 Feb 2023 23:57:22 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Content-Language: en-US-large To: ffmpeg-devel@ffmpeg.org References: <20230202150938.22297-1-leo.izen@gmail.com> <20230202150938.22297-3-leo.izen@gmail.com> From: Leo Izen In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH v3 2/3] avcodec/pngdec: read colorspace info when decoding with AVDISCARD_ALL 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/23 22:47, Andreas Rheinhardt wrote: > Leo Izen: >> These chunks are lightweight and it's useful information to have when >> running ffmpeg -i or ffprobe, for example. >> --- >> libavcodec/pngdec.c | 103 ++++++++++++++++++----------------- >> tests/ref/fate/png-icc | 8 +-- >> tests/ref/fate/png-side-data | 2 +- >> 3 files changed, 59 insertions(+), 54 deletions(-) >> >> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c >> index 0d969decf2..a80e0d15bb 100644 >> --- a/libavcodec/pngdec.c >> +++ b/libavcodec/pngdec.c >> @@ -1203,7 +1203,7 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, >> >> if (avctx->codec_id == AV_CODEC_ID_PNG && >> avctx->skip_frame == AVDISCARD_ALL) { >> - return 0; >> + goto exit_loop; >> } >> >> if (CONFIG_APNG_DECODER && avctx->codec_id == AV_CODEC_ID_APNG && length == 0) { >> @@ -1256,6 +1256,9 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, >> case MKTAG('t', 'E', 'X', 't'): >> case MKTAG('I', 'D', 'A', 'T'): >> case MKTAG('t', 'R', 'N', 'S'): >> + case MKTAG('s', 'R', 'G', 'B'): >> + case MKTAG('c', 'I', 'C', 'P'): >> + case MKTAG('c', 'H', 'R', 'M'): >> break; >> default: >> continue; >> @@ -1382,6 +1385,56 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, >> } >> exit_loop: >> >> + if (s->have_cicp) { >> + if (s->cicp_primaries >= AVCOL_PRI_NB) >> + av_log(avctx, AV_LOG_WARNING, "unrecognized cICP primaries\n"); >> + else >> + avctx->color_primaries = p->color_primaries = s->cicp_primaries; >> + if (s->cicp_trc >= AVCOL_TRC_NB) >> + av_log(avctx, AV_LOG_WARNING, "unrecognized cICP transfer\n"); >> + else >> + avctx->color_trc = p->color_trc = s->cicp_trc; > > This code (both the new code as well as the old code in output_frame() > added in 6f79f0971e8 and f7bab37c8e6 and 2548c32cc10) modify avctx > fields after ff_thread_finish_setup() and are racy with respect to the > generic copying of AVCodecContext fields in update_context_from_thread() > in pthread_frame.c. > Where should I update these fields instead to prevent races? I'm not sure where else to set it. >> + /* we don't support tv-range RGB */ >> + avctx->color_range = p->color_range = AVCOL_RANGE_JPEG; > > You set this unconditionally below, too. Shouldn't be set twice, good catch. Can remove this one. > >> + if (s->cicp_range == 0) >> + av_log(avctx, AV_LOG_WARNING, "unsupported tv-range cICP chunk\n"); >> + } else if (s->iccp_data) { >> + AVFrameSideData *sd = av_frame_new_side_data(p, AV_FRAME_DATA_ICC_PROFILE, s->iccp_data_len); >> + if (!sd) { >> + ret = AVERROR(ENOMEM); >> + goto fail; >> + } >> + memcpy(sd->data, s->iccp_data, s->iccp_data_len); >> + >> + av_dict_set(&sd->metadata, "name", s->iccp_name, 0); >> + } else if (s->have_srgb) { >> + avctx->color_primaries = p->color_primaries = AVCOL_PRI_BT709; >> + avctx->color_trc = p->color_trc = AVCOL_TRC_IEC61966_2_1; >> + } else if (s->have_chrm) { >> + AVColorPrimariesDesc desc; >> + enum AVColorPrimaries prim; >> + desc.wp.x = av_make_q(s->white_point[0], 100000); >> + desc.wp.y = av_make_q(s->white_point[1], 100000); >> + desc.prim.r.x = av_make_q(s->display_primaries[0][0], 100000); >> + desc.prim.r.y = av_make_q(s->display_primaries[0][1], 100000); >> + desc.prim.g.x = av_make_q(s->display_primaries[1][0], 100000); >> + desc.prim.g.y = av_make_q(s->display_primaries[1][1], 100000); >> + desc.prim.b.x = av_make_q(s->display_primaries[2][0], 100000); >> + desc.prim.b.y = av_make_q(s->display_primaries[2][1], 100000); >> + prim = av_csp_primaries_id_from_desc(&desc); >> + if (prim != AVCOL_PRI_UNSPECIFIED) >> + avctx->color_primaries = p->color_primaries = prim; >> + else >> + av_log(avctx, AV_LOG_WARNING, "unknown cHRM primaries\n"); >> + } >> + >> + /* these chunks override gAMA */ >> + if (s->iccp_data || s->have_srgb || s->have_cicp) >> + av_dict_set(&s->frame_metadata, "gamma", NULL, 0); >> + >> + avctx->colorspace = p->colorspace = AVCOL_SPC_RGB; >> + avctx->color_range = p->color_range = AVCOL_RANGE_JPEG; > > This is new in this patch; it changes behaviour even when nothing is > discarded. Is this intended? Yes. These are the only values supported by our PNG decoder so it makes sense to set them unconditionally. The color range is already set in the init function, but it's also set here for clarity. I could possibly move the unconditional colorspace setting to the init function as well and remove both of these lines. - Leo Izen (thebombzen) _______________________________________________ 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".