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 A9ACD4096E for ; Sat, 1 Oct 2022 22:23:51 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1EBD968B9A5; Sun, 2 Oct 2022 01:23:49 +0300 (EEST) Received: from mail-lf1-f48.google.com (mail-lf1-f48.google.com [209.85.167.48]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D2C0568B9A5 for ; Sun, 2 Oct 2022 01:23:42 +0300 (EEST) Received: by mail-lf1-f48.google.com with SMTP id q14so5050338lfo.11 for ; Sat, 01 Oct 2022 15:23:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date; bh=etaK45ZPDdsn2noW7tgnce9RWdvzBcEko1ap0CeA0KY=; b=UGpGnSvjlvAzA+hQnhcfrt9iKPptCm27eU382aIoPKB+FF0Pyh0k7zcQFkeCVGoUwL 7/0nZyV9FA+fPdMEjvi/15vugXt4G2kiGWhkqSSvFYaZpfplnUdiHxXXP0eLaSur/zzq DHBGCnCUCjZiDytMbA3kiO5sWoFQLJNevL5AXhpiOvPZw/vHKzXc/RSDyKru7H+1YOEt XLLrNc8waX0aRsIN7AbgnrG307ntqc/Hw9D434o+5mArN7sjRN2eejYRbWCRU4IzthHV nhpSkvj9SaaLgXtluLpffWMZKsqztsYSq4Z3ObQzVmZ6zlcGba0b0BVS0eyP5l689jc2 /zYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date; bh=etaK45ZPDdsn2noW7tgnce9RWdvzBcEko1ap0CeA0KY=; b=IRuNOTU3tzA1f38xaL5kyl+zjrwUwjdm/jPNeYhEOSRBL1YjEHjd9LvlGfIAF5VeaM 6hr1xSxBTqJPEiVdKPc9ExBEKHhuOpEovaonPj0aiUZWyKgwv8k9KgL1ri4RpgmHRYIK jIpuCTqK36PYxyFg4yOjWvl2V6qFsDRpDtGbHzlj3NDICNUwqWdnqm7/5p6udeNWDRwT tP2LXb3c3ewVe81YDD3EXxevQNyi/7fLpFeOx4VtryI0kzdfep5uBwtj2MuZjrd//0OK nlqE62DwWUWEfz1HqieTxi11jBwyiyHUtY2m5B2SdIN2sAsWeDae3KYLVk7tusKcY1Ku kzqw== X-Gm-Message-State: ACrzQf1c0A13UKDJYTVeNdR74jHyaxYzHqkbgCshDDZXPVte1btkVMFv VvlzJmOrvyjWxopXh4BXK4BX+DqTrwkRiAXSRd+fZ1o1/VI= X-Google-Smtp-Source: AMsMyM749VJ2zdho9i8+Osg2/6pIFBmAh5RtKy7GlkH+xWpzs4A3SpxnhRPbOC0jfPdwF1SrNLl9fhDuZhXwpuTYoO0= X-Received: by 2002:a05:6512:3091:b0:49a:d800:2828 with SMTP id z17-20020a056512309100b0049ad8002828mr5370653lfd.534.1664663019843; Sat, 01 Oct 2022 15:23:39 -0700 (PDT) MIME-Version: 1.0 References: <20221001140633.1722-1-mindmark@gmail.com> In-Reply-To: From: Mark Reid Date: Sat, 1 Oct 2022 15:23:27 -0700 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [FFmpeg-devel] [PATCH] avcodec/tiff: add support for decoding compressed rgb floating point formats 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Sat, Oct 1, 2022 at 7:22 AM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > mindmark@gmail.com: > > From: Mark Reid > > > > floating point uses a slightly different predictor technique describe > here > > http://chriscox.org/TIFFTN3d1.pdf > > > > Here is a link the test files, if someone could add them to fate me > > https://www.dropbox.com/s/fg59h2os4gb4wug/tiff_fate_samples.zip > > > > > > --- > > libavcodec/tiff.c | 66 +++++++++++++++++++++- > > tests/fate/image.mak | 18 ++++++ > > tests/ref/fate/tiff-lzw-rgbaf32le | 6 ++ > > tests/ref/fate/tiff-lzw-rgbf32le | 6 ++ > > tests/ref/fate/tiff-uncompressed-rgbaf32le | 6 ++ > > tests/ref/fate/tiff-uncompressed-rgbf32le | 6 ++ > > tests/ref/fate/tiff-zip-rgbaf32le | 6 ++ > > tests/ref/fate/tiff-zip-rgbf32le | 6 ++ > > 8 files changed, 119 insertions(+), 1 deletion(-) > > create mode 100644 tests/ref/fate/tiff-lzw-rgbaf32le > > create mode 100644 tests/ref/fate/tiff-lzw-rgbf32le > > create mode 100644 tests/ref/fate/tiff-uncompressed-rgbaf32le > > create mode 100644 tests/ref/fate/tiff-uncompressed-rgbf32le > > create mode 100644 tests/ref/fate/tiff-zip-rgbaf32le > > create mode 100644 tests/ref/fate/tiff-zip-rgbf32le > > > > diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c > > index 3a610ada85..c1d07f8c3e 100644 > > --- a/libavcodec/tiff.c > > +++ b/libavcodec/tiff.c > > @@ -1913,7 +1913,7 @@ static int decode_frame(AVCodecContext *avctx, > AVFrame *p, > > TiffContext *const s = avctx->priv_data; > > unsigned off, last_off = 0; > > int le, ret, plane, planes; > > - int i, j, entries, stride; > > + int i, j, k, entries, stride; > > Don't add another loop variable. We support "for (int k = 0;" nowadays. > > > unsigned soff, ssize; > > uint8_t *dst; > > GetByteContext stripsizes; > > @@ -2249,6 +2249,70 @@ again: > > } > > } > > > > + /* Floating point predictor > > + TIFF Technical Note 3 http://chriscox.org/TIFFTN3d1.pdf */ > > + if (s->predictor == 3) { > > + int channels = s->bppcount; > > + int lane_offset; > > + uint8_t *tmpbuf; > > + int bpc; > > + > > + dst = five_planes ? five_planes : p->data[plane]; > > + soff = s->bpp >> 3; > > + if (s->planar) { > > + soff = FFMAX(soff / s->bppcount, 1); > > + channels = 1; > > + } > > + ssize = s->width * soff; > > + bpc = FFMAX(soff / s->bppcount, 1); /* bytes per component > */ > > + lane_offset = s->width * channels; > > + > > + tmpbuf = (uint8_t*)av_malloc(ssize); > > unnecessary cast. > > > + if (!tmpbuf) > > + return AVERROR(ENOMEM); > > + > > + if (s->avctx->pix_fmt == AV_PIX_FMT_RGBF32LE || > > + s->avctx->pix_fmt == AV_PIX_FMT_RGBAF32LE) { > > + for (i = 0; i < decoded_height; i++) { > > + /* copy first sample byte for each channel */ > > + for (j = 0; j < channels; j++) > > + tmpbuf[j] = dst[j]; > > + > > + /* decode horizontal differences */ > > + for (j = channels; j < ssize; j++) > > + tmpbuf[j] = dst[j] + tmpbuf[j-channels]; > > + > > + /* combine shuffled bytes from their sepearate > lanes */ > > ^ > > Did you not like this comment? I can remove it, I felt the code needed some explanation and I don't think the comment is the best. The docs example code refers to these lanes as rowIncrements, which I thought was a confusing name too. Each byte of every floating point value in a row of pixels is split and combined into separate groups. A group of all the sign/exponents bytes in the row and groups for each of the upper, mid, and lower mantissa bytes in the row. Maybe group_size is a better name than lane_offset now that I typed that out. > > + for (j = 0; j < lane_offset; j++) { > > + for (k = 0; k < bpc; k++) { > > + dst[bpc * j + k] = tmpbuf[(bpc - k - 1) * > lane_offset + j]; > > + } > > + } > > + dst += stride; > > + } > > + } else if (s->avctx->pix_fmt == AV_PIX_FMT_RGBF32BE || > > + s->avctx->pix_fmt == AV_PIX_FMT_RGBAF32BE) { > > + /* same as LE only the shuffle at the end is reversed */ > > + for (i = 0; i < decoded_height; i++) { > > + for (j = 0; j < channels; j++) > > + tmpbuf[j] = dst[j]; > > + > > + for (j = channels; j < ssize; j++) > > + tmpbuf[j] = dst[j] + tmpbuf[j-channels]; > > + > > + for (j = 0; j < lane_offset; j++) { > > + for (k = 0; k < bpc; k++) { > > + dst[bpc * j + k] = tmpbuf[k * lane_offset + > j]; > > + } > > + } > > + dst += stride; > > + } > > + } else { > > + av_log(s->avctx, AV_LOG_ERROR, "unsupported floating > point pixel format\n"); > > + } > > + av_free(tmpbuf); > > + } > > + > > if (s->photometric == TIFF_PHOTOMETRIC_WHITE_IS_ZERO) { > > int c = (s->avctx->pix_fmt == AV_PIX_FMT_PAL8 ? (1<bpp) > - 1 : 255); > > dst = p->data[plane]; > > diff --git a/tests/fate/image.mak b/tests/fate/image.mak > > index 03e794dc48..971531520d 100644 > > --- a/tests/fate/image.mak > > +++ b/tests/fate/image.mak > > @@ -501,6 +501,24 @@ fate-tiff-fax-g3: CMD = framecrc -i > $(TARGET_SAMPLES)/CCITT_fax/G31D.TIF > > FATE_TIFF += fate-tiff-fax-g3s > > fate-tiff-fax-g3s: CMD = framecrc -i > $(TARGET_SAMPLES)/CCITT_fax/G31DS.TIF > > > > +FATE_TIFF += fate-tiff-uncompressed-rgbf32le > > +fate-tiff-uncompressed-rgbf32le: CMD = framecrc -i > $(TARGET_SAMPLES)/tiff/uncompressed_rgbf32le.tif > > + > > +FATE_TIFF += fate-tiff-uncompressed-rgbaf32le > > +fate-tiff-uncompressed-rgbaf32le: CMD = framecrc -i > $(TARGET_SAMPLES)/tiff/uncompressed_rgbaf32le.tif > > + > > +FATE_TIFF += fate-tiff-lzw-rgbf32le > > +fate-tiff-lzw-rgbf32le: CMD = framecrc -i > $(TARGET_SAMPLES)/tiff/lzw_rgbf32le.tif > > + > > +FATE_TIFF += fate-tiff-lzw-rgbaf32le > > +fate-tiff-lzw-rgbaf32le: CMD = framecrc -i > $(TARGET_SAMPLES)/tiff/lzw_rgbaf32le.tif > > + > > +FATE_TIFF += fate-tiff-zip-rgbf32le > > +fate-tiff-zip-rgbf32le: CMD = framecrc -i > $(TARGET_SAMPLES)/tiff/zip_rgbf32le.tif > > + > > +FATE_TIFF += fate-tiff-zip-rgbaf32le > > +fate-tiff-zip-rgbaf32le: CMD = framecrc -i > $(TARGET_SAMPLES)/tiff/zip_rgbaf32le.tif > > + > > FATE_TIFF-$(call DEMDEC, IMAGE2, TIFF) += $(FATE_TIFF) > > I already wanted to write that framecrc tests don't really work with > floating point formats, but then I saw that there is no real floating > point code in this patch. Very weird for a floating point format. > (Apart from that: You want to use FRAMECRC instead of DEMDEC.) > > yes there is no floating point calculations involved, the process should always be bitexact. you mean like this right? FATE_TIFF-$(call FRAMECRC, IMAGE2, TIFF) += $(FATE_TIFF) Thanks for the quick review! > > > FATE_IMAGE_FRAMECRC += $(FATE_TIFF-yes) > > diff --git a/tests/ref/fate/tiff-lzw-rgbaf32le > b/tests/ref/fate/tiff-lzw-rgbaf32le > > new file mode 100644 > > index 0000000000..c99aa02ef0 > > --- /dev/null > > +++ b/tests/ref/fate/tiff-lzw-rgbaf32le > > @@ -0,0 +1,6 @@ > > +#tb 0: 1/25 > > +#media_type 0: video > > +#codec_id 0: rawvideo > > +#dimensions 0: 8x8 > > +#sar 0: 0/1 > > +0, 0, 0, 1, 1024, 0x877e1d5f > > diff --git a/tests/ref/fate/tiff-lzw-rgbf32le > b/tests/ref/fate/tiff-lzw-rgbf32le > > new file mode 100644 > > index 0000000000..a6d3fabfda > > --- /dev/null > > +++ b/tests/ref/fate/tiff-lzw-rgbf32le > > @@ -0,0 +1,6 @@ > > +#tb 0: 1/25 > > +#media_type 0: video > > +#codec_id 0: rawvideo > > +#dimensions 0: 8x8 > > +#sar 0: 0/1 > > +0, 0, 0, 1, 768, 0xad26ed90 > > diff --git a/tests/ref/fate/tiff-uncompressed-rgbaf32le > b/tests/ref/fate/tiff-uncompressed-rgbaf32le > > new file mode 100644 > > index 0000000000..c99aa02ef0 > > --- /dev/null > > +++ b/tests/ref/fate/tiff-uncompressed-rgbaf32le > > @@ -0,0 +1,6 @@ > > +#tb 0: 1/25 > > +#media_type 0: video > > +#codec_id 0: rawvideo > > +#dimensions 0: 8x8 > > +#sar 0: 0/1 > > +0, 0, 0, 1, 1024, 0x877e1d5f > > diff --git a/tests/ref/fate/tiff-uncompressed-rgbf32le > b/tests/ref/fate/tiff-uncompressed-rgbf32le > > new file mode 100644 > > index 0000000000..a6d3fabfda > > --- /dev/null > > +++ b/tests/ref/fate/tiff-uncompressed-rgbf32le > > @@ -0,0 +1,6 @@ > > +#tb 0: 1/25 > > +#media_type 0: video > > +#codec_id 0: rawvideo > > +#dimensions 0: 8x8 > > +#sar 0: 0/1 > > +0, 0, 0, 1, 768, 0xad26ed90 > > diff --git a/tests/ref/fate/tiff-zip-rgbaf32le > b/tests/ref/fate/tiff-zip-rgbaf32le > > new file mode 100644 > > index 0000000000..c99aa02ef0 > > --- /dev/null > > +++ b/tests/ref/fate/tiff-zip-rgbaf32le > > @@ -0,0 +1,6 @@ > > +#tb 0: 1/25 > > +#media_type 0: video > > +#codec_id 0: rawvideo > > +#dimensions 0: 8x8 > > +#sar 0: 0/1 > > +0, 0, 0, 1, 1024, 0x877e1d5f > > diff --git a/tests/ref/fate/tiff-zip-rgbf32le > b/tests/ref/fate/tiff-zip-rgbf32le > > new file mode 100644 > > index 0000000000..a6d3fabfda > > --- /dev/null > > +++ b/tests/ref/fate/tiff-zip-rgbf32le > > @@ -0,0 +1,6 @@ > > +#tb 0: 1/25 > > +#media_type 0: video > > +#codec_id 0: rawvideo > > +#dimensions 0: 8x8 > > +#sar 0: 0/1 > > +0, 0, 0, 1, 768, 0xad26ed90 > > -- > > 2.31.1.windows.1 > > > > _______________________________________________ > 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". > _______________________________________________ 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".