| Message ID | 20221001140633.1722-1-mindmark@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/tiff: add support for decoding compressed rgb floating point formats | expand |
mindmark@gmail.com: > From: Mark Reid <mindmark@gmail.com> > > 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 */ ^ > + 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<<s->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.) > > 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 >
On Sat, Oct 1, 2022 at 7:22 AM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > mindmark@gmail.com: > > From: Mark Reid <mindmark@gmail.com> > > > > 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<<s->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". >
Mark Reid: > On Sat, Oct 1, 2022 at 7:22 AM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> mindmark@gmail.com: >>> From: Mark Reid <mindmark@gmail.com> >>> >>> 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. There is a typo: sepearate should be separate. > 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<<s->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) Yes. > > 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 >>>
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; 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); + 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 */ + 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<<s->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) 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
From: Mark Reid <mindmark@gmail.com> 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 -- 2.31.1.windows.1