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 84672465C2 for ; Thu, 22 Jun 2023 00:59:35 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 89E9E68C077; Thu, 22 Jun 2023 03:59:32 +0300 (EEST) Received: from mail-ot1-f51.google.com (mail-ot1-f51.google.com [209.85.210.51]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A164368B121 for ; Thu, 22 Jun 2023 03:59:25 +0300 (EEST) Received: by mail-ot1-f51.google.com with SMTP id 46e09a7af769-6b5d5e6b086so87315a34.1 for ; Wed, 21 Jun 2023 17:59:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687395564; x=1689987564; 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=Cy3RYhc2k/De04GID3fCR4OzXKBKqGln9TLLZ6JE2Vk=; b=rK+7NhFKs2thjIqShAqEmJWFF/bDG94JSNrS048jdBH9xZ7iELKjq7OlromO7HPzKt Ay2athz/05FaIZ8FB4upwirIw04SIapfoSHm4qLexP9m6Ac15IFhLcBS/+V0Yo6yk6Tz o/WpzFWkic6ThqQoZ7/ZhPFn5SYkTHvKW/87JWAKCwMmykj8JO7Tw6nb070ngEpKmKSC VLR/G+fzmjqyWBGBavjSr90HL72X4MChdMnSv1gSDep5w8WT6/aeTQ4YI02WTcvr6UVB JOFOdgRHuArQAUTZbcTFSVv+EW3t1lqKH94p/xYv9OeYVcoGwsV4q5cVq2xtbzI0MFI+ ve7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687395564; x=1689987564; 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=Cy3RYhc2k/De04GID3fCR4OzXKBKqGln9TLLZ6JE2Vk=; b=XQYJV9RHsqwNs++XqhirULfq24zjKTO2JGyxgJzHspLEOxE6d9qVkMOyAkl5kWC6z9 nmq9VnRO7GoJYJPPGnA/AUGAoJLLpHT/J0d2eiE5gX4JvR88LMoE7L+KYtgpdCyn2KVM yylvy/PHzipvlDR3xDWwczk40dRPMJvSTZPYtCnEvJ+/9ppP9yRd0CoPphnSTemnFMxM YlUZzUEUnv/YMALMQSEfnrgkISbeLSVUmNcxeRjHgfUKqIUpTzFhrEc4Dp15L5u50zfS t3QPbwPUiVI8+M4jlPYxv18C94Zsvk2fOc1TXk30ocfyvrjj+uJ1UBU2ScV154omjwlE TuKQ== X-Gm-Message-State: AC+VfDxxrc140cYSbmzJlfIq9kZAUqh+C/+m4uuypxuz3RgarFQ0ENZi 0kpJjN2tzqU5k1zvoGIwfbmhofi+dYI= X-Google-Smtp-Source: ACHHUZ7ZP/vvhfj/dUWsdVBlcIY7ilUT0WmvsZG6bP1+rGXv47jwUEWO1J2nJDvlGs2LwnEnjCk05w== X-Received: by 2002:a9d:6498:0:b0:6af:8a2b:553d with SMTP id g24-20020a9d6498000000b006af8a2b553dmr7365530otl.0.1687395563607; Wed, 21 Jun 2023 17:59:23 -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 h20-20020a9d6f94000000b006b4687d1173sm2401937otq.56.2023.06.21.17.59.22 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 21 Jun 2023 17:59:23 -0700 (PDT) Message-ID: <1828a416-62ca-9cf2-d774-845574a88796@gmail.com> Date: Wed, 21 Jun 2023 21:59:35 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20230622004313.69697-1-leo.izen@gmail.com> <20230622004313.69697-2-leo.izen@gmail.com> From: James Almer In-Reply-To: <20230622004313.69697-2-leo.izen@gmail.com> Subject: Re: [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser 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/21/2023 9:43 PM, Leo Izen wrote: > diff --git a/libavcodec/jpegxl_parse.c b/libavcodec/jpegxl_parse.c > new file mode 100644 > index 0000000000..be360acb08 > --- /dev/null > +++ b/libavcodec/jpegxl_parse.c > @@ -0,0 +1,22 @@ > +/* > + * JPEG XL Header Parser > + * Copyright (c) 2023 Leo Izen > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#include "jpegxl_parse.h" This is a really weird way to achieve code sharing between libraries. What you named jpegxl_parse.h should be jpegxl_parse.c instead, and jpegxl_parse.h should only have the prototypes for ff_jpegxl_collect_codestream_header() and ff_jpegxl_parse_codestream_header(), plus the definition of FFJXLMetadata. The enums can stay in jpegxl.h, and they for that matter don't need the FF_ prefix. > diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c > new file mode 100644 > index 0000000000..096b22be0f > --- /dev/null > +++ b/libavcodec/jpegxl_parser.c > @@ -0,0 +1,165 @@ > +/* > + * JPEG XL Codec Parser > + * Copyright (c) 2023 Leo Izen > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#include "libavutil/intreadwrite.h" > + > +#include "codec_id.h" > +#include "jpegxl.h" > +#include "parser.h" > + > +typedef struct JXLParseContext { > + ParseContext pc; You don't need this. You're not assembling a packet. > + FFJXLMetadata meta; > + int parsed_header; > +} JXLParseContext; > + > +static int jpegxl_parse(AVCodecParserContext *s, AVCodecContext *avctx, > + const uint8_t **poutbuf, int *poutbuf_size, > + const uint8_t *buf, int buf_size) > +{ > + JXLParseContext *ctx = s->priv_data; > + int ret; > + > + *poutbuf_size = 0; > + *poutbuf = NULL; > + > + if (!ctx->parsed_header) { > + if (AV_RL64(buf) == FF_JPEGXL_CONTAINER_SIGNATURE_LE) { > + int copied; > + uint8_t codestream_header[4096]; > + ret = ff_jpegxl_collect_codestream_header(buf, buf_size, codestream_header, > + sizeof(codestream_header), &copied); > + if (ret < 0) > + return ret; > + /* copied may be larger than the bufsize if stuff was skipped */ > + copied = FFMIN(copied, sizeof(codestream_header)); > + ret = ff_jpegxl_parse_codestream_header(codestream_header, copied, 0, &ctx->meta); > + if (ret < 0) > + return ret; > + } else { > + ret = ff_jpegxl_parse_codestream_header(buf, buf_size, 1, &ctx->meta); > + if (ret < 0) > + return ret; > + } > + avctx->width = ctx->meta.width; > + avctx->height = ctx->meta.height; > + > + switch (ctx->meta.csp) { > + case FF_JPEGXL_CS_RGB: > + case FF_JPEGXL_CS_XYB: > + avctx->colorspace = AVCOL_SPC_RGB; > + break; > + default: > + avctx->colorspace = AVCOL_SPC_UNSPECIFIED; > + } > + > + if (ctx->meta.wp == FF_JPEGXL_WP_D65) { > + switch (ctx->meta.primaries) { > + case FF_JPEGXL_PR_SRGB: > + avctx->color_primaries = AVCOL_PRI_BT709; > + break; > + case FF_JPEGXL_PR_P3: > + avctx->color_primaries = AVCOL_PRI_SMPTE432; > + break; > + case FF_JPEGXL_PR_2100: > + avctx->color_primaries = AVCOL_PRI_BT2020; > + break; > + default: > + avctx->color_primaries = AVCOL_PRI_UNSPECIFIED; > + } > + } else if (ctx->meta.wp == FF_JPEGXL_WP_DCI && ctx->meta.primaries == FF_JPEGXL_PR_P3) { > + avctx->color_primaries = AVCOL_PRI_SMPTE431; > + } else { > + avctx->color_primaries = AVCOL_PRI_UNSPECIFIED; > + } > + > + if (ctx->meta.trc > FF_JPEGXL_TR_GAMMA) { > + FFJXLTransferCharacteristic trc = ctx->meta.trc - FF_JPEGXL_TR_GAMMA; > + switch (trc) { > + case FF_JPEGXL_TR_BT709: > + avctx->color_trc = AVCOL_TRC_BT709; > + break; > + case FF_JPEGXL_TR_LINEAR: > + avctx->color_trc = AVCOL_TRC_LINEAR; > + break; > + case FF_JPEGXL_TR_SRGB: > + avctx->color_trc = AVCOL_TRC_IEC61966_2_1; > + break; > + case FF_JPEGXL_TR_PQ: > + avctx->color_trc = AVCOL_TRC_SMPTEST2084; > + break; > + case FF_JPEGXL_TR_DCI: > + avctx->color_trc = AVCOL_TRC_SMPTE428; > + break; > + case FF_JPEGXL_TR_HLG: > + avctx->color_trc = AVCOL_TRC_ARIB_STD_B67; > + break; > + default: > + avctx->color_trc = AVCOL_TRC_UNSPECIFIED; > + } > + } else if (ctx->meta.trc > 0) { > + if (ctx->meta.trc > 45355 && ctx->meta.trc < 45555) > + avctx->color_trc = AVCOL_TRC_GAMMA22; > + else if (ctx->meta.trc > 35614 && ctx->meta.trc < 35814) > + avctx->color_trc = AVCOL_TRC_GAMMA28; > + else > + avctx->color_trc = AVCOL_TRC_UNSPECIFIED; > + } else { > + avctx->color_trc = AVCOL_TRC_UNSPECIFIED; > + } > + > + if (ctx->meta.csp == FF_JPEGXL_CS_GRAY) { > + if (ctx->meta.bit_depth <= 8) > + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_YA8 : AV_PIX_FMT_GRAY8;. Set s->format, not avctx->pix_fmt. The format in an AVCodecContext is set by a decoder, which may not match what the parser sets. > + else if (ctx->meta.bit_depth <= 16) > + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_YA16 : AV_PIX_FMT_GRAY16; > + else > + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_NONE : AV_PIX_FMT_GRAYF32; > + } else { > + if (ctx->meta.bit_depth <= 8) > + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_RGBA : AV_PIX_FMT_RGB24; > + else if (ctx->meta.bit_depth <= 16) > + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_RGBA64 : AV_PIX_FMT_RGB48; > + else > + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_RGBAF32 : AV_PIX_FMT_RGBF32; > + } > + > + ctx->parsed_header = 1; So this is done once for the entire stream? > + } > + > + ctx->pc.frame_start_found = 1; Unnecessary. > + > + ret = ff_combine_frame(&ctx->pc, END_NOT_FOUND, &buf, &buf_size); Also unnecessary. > + if (ret < 0) > + return buf_size; > + > + *poutbuf = buf; > + *poutbuf_size = buf_size; > + > + return END_NOT_FOUND; ??? The process succeeded. Return 0. > +} > + > +const AVCodecParser ff_jpegxl_parser = { > + .codec_ids = { AV_CODEC_ID_JPEGXL }, > + .priv_data_size = sizeof(JXLParseContext), > + .parser_parse = jpegxl_parse, > + .parser_close = ff_parse_close, > +}; _______________________________________________ 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".