| Message ID | b883e7aa-2e84-23b2-9f76-21b2242e80a7@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Am Do., 3. Okt. 2019 um 20:50 Uhr schrieb Skakov Pavel <pavelsx@gmail.com>: > > Add support for properly handling PC/TV ranges and Rec601/Rec709 color spaces. Can't this be implemented without using floats? Carl Eugen
On 10/3/2019 3:42 PM, Skakov Pavel wrote: > Add support for properly handling PC/TV ranges and Rec601/Rec709 color > spaces. > Example for PC range YUV, compare to ImageMagick decoding: > https://samples.ffmpeg.org/image-samples/dscf0013.tif > > 0001-avcodec-tiff-add-limited-support-for-ReferenceBlackW.patch > > From 5e24edbf73f3a897fd203e36963e9cf5db5e688d Mon Sep 17 00:00:00 2001 > From: Pavel Skakov <pavelsx@gmail.com> > Date: Thu, 3 Oct 2019 21:28:10 +0300 > Subject: [PATCH] avcodec/tiff: add limited support for ReferenceBlackWhite and > YCbCrCoefficients tags > > Signed-off-by: Pavel Skakov <pavelsx@gmail.com> > --- > libavcodec/tiff.c | 74 ++++++++++++++++++++++++++++++++++++++++-- > tests/ref/fate/exif-image-tiff | 2 +- > 2 files changed, 73 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c > index 8b39ca1ebc..90215fce09 100644 > --- a/libavcodec/tiff.c > +++ b/libavcodec/tiff.c > @@ -1441,12 +1441,16 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame) > break; > case TIFF_PHOTOMETRIC: > switch (value) { > + case TIFF_PHOTOMETRIC_YCBCR: > + s->avctx->colorspace = AVCOL_SPC_BT470BG; > + // fallthrough > + case TIFF_PHOTOMETRIC_RGB: > + s->avctx->color_range = AVCOL_RANGE_JPEG; > + // fallthrough > case TIFF_PHOTOMETRIC_WHITE_IS_ZERO: > case TIFF_PHOTOMETRIC_BLACK_IS_ZERO: > - case TIFF_PHOTOMETRIC_RGB: > case TIFF_PHOTOMETRIC_PALETTE: > case TIFF_PHOTOMETRIC_SEPARATED: > - case TIFF_PHOTOMETRIC_YCBCR: > case TIFF_PHOTOMETRIC_CFA: > case TIFF_PHOTOMETRIC_LINEAR_RAW: // Used by DNG images > s->photometric = value; > @@ -1519,6 +1523,72 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame) > } > } > break; > + case TIFF_YCBCR_COEFFICIENTS: > + if (s->photometric == TIFF_PHOTOMETRIC_YCBCR) > + if (count != 3 || type != TIFF_RATIONAL) { > + av_log(s->avctx, AV_LOG_ERROR, "YCbCrCoefficients are invalid\n"); > + return AVERROR_INVALIDDATA; > + } else { > + // LibTIFF handles rational values by converting them to/from floats, so we do the same in tests for equality > + float coef[3]; You should use AVRationals instead. It's ideal seeing the way these values are coded in the bitstream. See libavutil/rational.h > + for (i = 0; i < 3; i++) { > + unsigned num = ff_tget(&s->gb, TIFF_LONG, s->le); > + unsigned den = ff_tget(&s->gb, TIFF_LONG, s->le); > + if (!den) { > + av_log(s->avctx, AV_LOG_ERROR, "YCbCrCoefficients divisor is zero\n"); > + return AVERROR_INVALIDDATA; > + } > + coef[i] = (float)num/den; coef[i] = av_make_q(num, den); > + } > + if (coef[0] == 0.2125f && coef[1] == 0.7154f && coef[2] == 0.0721f) if (av_cmp_q(coef[0], (AVRational) { 17, 80 }) && ...) > + s->avctx->colorspace = AVCOL_SPC_BT709; > + else if (coef[0] != 0.299f || coef[1] != 0.587f || coef[2] != 0.114f) { > + av_log(s->avctx, AV_LOG_WARNING, "Unrecognized YCbCrCoefficients values: %.4f %.4f %.4f\n", coef[0], coef[1], coef[2]); av_log(... "%.4f %.4f %.4f\n", av_q2d(coef[0]), av_q2d(coef[1]), av_q2d(coef[2])); etc. > + s->avctx->colorspace = AVCOL_SPC_UNSPECIFIED; > + } > + } > + break; > + case TIFF_REFERENCE_BW: > + if (s->photometric == TIFF_PHOTOMETRIC_YCBCR || s->photometric == TIFF_PHOTOMETRIC_RGB) > + if (count != 6 || type != TIFF_RATIONAL) { > + av_log(s->avctx, AV_LOG_ERROR, "ReferenceBlackWhite is invalid\n"); > + return AVERROR_INVALIDDATA; > + } else { > + unsigned bpp = s->bpp/s->bppcount; > + unsigned mul = 1 << (bpp - 8); > + float max_val = (1 << bpp) - 1; > + float coef[6]; > + for (i = 0; i < 6; i++) { > + unsigned num = ff_tget(&s->gb, TIFF_LONG, s->le); > + unsigned den = ff_tget(&s->gb, TIFF_LONG, s->le); > + if (!den) { > + av_log(s->avctx, AV_LOG_ERROR, "ReferenceBlackWhite divisor is zero\n"); > + return AVERROR_INVALIDDATA; > + } > + coef[i] = (float)num/den; > + } > + if (s->photometric == TIFF_PHOTOMETRIC_YCBCR) { > + if (!coef[0] && coef[1] == max_val && coef[2] == (max_val + 1)/2 && coef[3] == max_val && coef[4] == coef[2] && coef[5] == coef[3]) > + s->avctx->color_range = AVCOL_RANGE_JPEG; > + // NOTE: TIFF 6.0 specification has an example where it mistakenly shows TV range coef[0] as 15 > + else if (coef[0] == 16*mul && coef[1] == 235*mul && coef[2] == 128*mul && coef[3] == 240*mul && coef[4] == coef[2] && coef[5] == coef[3]) > + s->avctx->color_range = AVCOL_RANGE_MPEG; > + else { > + av_log(s->avctx, AV_LOG_WARNING, "Unrecognized ReferenceBlackWhite values: [%g;%g] [%g;%g] [%g;%g]\n", coef[0], coef[1], coef[2], coef[3], coef[4], coef[5]); > + s->avctx->color_range = AVCOL_RANGE_UNSPECIFIED; > + } > + } else { > + if (!coef[0] && coef[1] == max_val && !coef[2] && coef[3] == max_val && !coef[4] && coef[5] == max_val) > + s->avctx->color_range = AVCOL_RANGE_JPEG; > + else if (coef[0] == 16*mul && coef[1] == 235*mul && coef[2] == coef[0] && coef[3] == coef[1] && coef[2] == coef[0] && coef[3] == coef[1]) > + s->avctx->color_range = AVCOL_RANGE_MPEG; > + else { > + av_log(s->avctx, AV_LOG_WARNING, "Unrecognized ReferenceBlackWhite values: [%g;%g] [%g;%g] [%g;%g]\n", coef[0], coef[1], coef[2], coef[3], coef[4], coef[5]); > + s->avctx->color_range = AVCOL_RANGE_UNSPECIFIED; > + } > + } > + } > + break; > case TIFF_T4OPTIONS: > if (s->compr == TIFF_G3) > s->fax_opts = value; > diff --git a/tests/ref/fate/exif-image-tiff b/tests/ref/fate/exif-image-tiff > index 51580601e1..ebf2f38d6e 100644 > --- a/tests/ref/fate/exif-image-tiff > +++ b/tests/ref/fate/exif-image-tiff > @@ -22,7 +22,7 @@ display_picture_number=0 > interlaced_frame=0 > top_field_first=0 > repeat_pict=0 > -color_range=unknown > +color_range=pc > color_space=unknown > color_primaries=unknown > color_transfer=unknown > -- 2.13.2.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". >
> You should use AVRationals instead. It's ideal seeing the way these > values are coded in the bitstream. > > See libavutil/rational.h Can AVRational be used even through the values are essentially unsigned? LibTIFF really likes to encode 0..1 values with denominator 0xFFFFFFFF.
On Fri, Oct 04, 2019 at 09:24:05AM +0300, Skakov Pavel wrote: > >You should use AVRationals instead. It's ideal seeing the way these > >values are coded in the bitstream. > > > >See libavutil/rational.h > > Can AVRational be used even through the values are essentially unsigned? LibTIFF really likes to encode 0..1 values with denominator 0xFFFFFFFF. floats do not have enough precission to store unsigned 32/32bit fractions just consider a float has 32bit (and a good part is really small and large numbers) the faction has 64bits with num+den. Thats not awnsering your question but rather i wonder why you ask it here instead of with floats for 32bit signed fractions. i think the difference wontg matter but you should probably test this ... Thanks [...]
>> Can AVRational be used even through the values are essentially unsigned? LibTIFF really likes to encode 0..1 values with denominator 0xFFFFFFFF. > > floats do not have enough precission to store unsigned 32/32bit fractions > just consider a float has 32bit (and a good part is really small and large > numbers) the faction has 64bits with num+den. > > Thats not awnsering your question but rather i wonder why you ask it here > instead of with floats > for 32bit signed fractions. i think the difference wontg matter but you > should probably test this ... Of course floats don't have enough precision for exact values but that's not the issue here as we only need to match the values (so some rounding is actually good). The point was that ffmpeg has AVRational that might be a cleaner approach at handling given rationals than matching the other software's way (floats).
On 10/4/2019 3:24 AM, Skakov Pavel wrote: >> You should use AVRationals instead. It's ideal seeing the way these >> values are coded in the bitstream. >> >> See libavutil/rational.h > > Can AVRational be used even through the values are essentially unsigned? > LibTIFF really likes to encode 0..1 values with denominator 0xFFFFFFFF. Is it feasible for this to use av_reduce() with INT_MAX as max argument? It takes int64_t as input values, so UINT_MAX should be fine. > _______________________________________________ > 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".
>> Can AVRational be used even through the values are essentially unsigned? >> LibTIFF really likes to encode 0..1 values with denominator 0xFFFFFFFF. > > Is it feasible for this to use av_reduce() with INT_MAX as max argument? > It takes int64_t as input values, so UINT_MAX should be fine. It seems to be able to handle the case, but that code looks like an overkill for what was required to be done. See PATCH v2 – I rewrote it with plain integer multiplications.
From 5e24edbf73f3a897fd203e36963e9cf5db5e688d Mon Sep 17 00:00:00 2001 From: Pavel Skakov <pavelsx@gmail.com> Date: Thu, 3 Oct 2019 21:28:10 +0300 Subject: [PATCH] avcodec/tiff: add limited support for ReferenceBlackWhite and YCbCrCoefficients tags Signed-off-by: Pavel Skakov <pavelsx@gmail.com> --- libavcodec/tiff.c | 74 ++++++++++++++++++++++++++++++++++++++++-- tests/ref/fate/exif-image-tiff | 2 +- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c index 8b39ca1ebc..90215fce09 100644 --- a/libavcodec/tiff.c +++ b/libavcodec/tiff.c @@ -1441,12 +1441,16 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame) break; case TIFF_PHOTOMETRIC: switch (value) { + case TIFF_PHOTOMETRIC_YCBCR: + s->avctx->colorspace = AVCOL_SPC_BT470BG; + // fallthrough + case TIFF_PHOTOMETRIC_RGB: + s->avctx->color_range = AVCOL_RANGE_JPEG; + // fallthrough case TIFF_PHOTOMETRIC_WHITE_IS_ZERO: case TIFF_PHOTOMETRIC_BLACK_IS_ZERO: - case TIFF_PHOTOMETRIC_RGB: case TIFF_PHOTOMETRIC_PALETTE: case TIFF_PHOTOMETRIC_SEPARATED: - case TIFF_PHOTOMETRIC_YCBCR: case TIFF_PHOTOMETRIC_CFA: case TIFF_PHOTOMETRIC_LINEAR_RAW: // Used by DNG images s->photometric = value; @@ -1519,6 +1523,72 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame) } } break; + case TIFF_YCBCR_COEFFICIENTS: + if (s->photometric == TIFF_PHOTOMETRIC_YCBCR) + if (count != 3 || type != TIFF_RATIONAL) { + av_log(s->avctx, AV_LOG_ERROR, "YCbCrCoefficients are invalid\n"); + return AVERROR_INVALIDDATA; + } else { + // LibTIFF handles rational values by converting them to/from floats, so we do the same in tests for equality + float coef[3]; + for (i = 0; i < 3; i++) { + unsigned num = ff_tget(&s->gb, TIFF_LONG, s->le); + unsigned den = ff_tget(&s->gb, TIFF_LONG, s->le); + if (!den) { + av_log(s->avctx, AV_LOG_ERROR, "YCbCrCoefficients divisor is zero\n"); + return AVERROR_INVALIDDATA; + } + coef[i] = (float)num/den; + } + if (coef[0] == 0.2125f && coef[1] == 0.7154f && coef[2] == 0.0721f) + s->avctx->colorspace = AVCOL_SPC_BT709; + else if (coef[0] != 0.299f || coef[1] != 0.587f || coef[2] != 0.114f) { + av_log(s->avctx, AV_LOG_WARNING, "Unrecognized YCbCrCoefficients values: %.4f %.4f %.4f\n", coef[0], coef[1], coef[2]); + s->avctx->colorspace = AVCOL_SPC_UNSPECIFIED; + } + } + break; + case TIFF_REFERENCE_BW: + if (s->photometric == TIFF_PHOTOMETRIC_YCBCR || s->photometric == TIFF_PHOTOMETRIC_RGB) + if (count != 6 || type != TIFF_RATIONAL) { + av_log(s->avctx, AV_LOG_ERROR, "ReferenceBlackWhite is invalid\n"); + return AVERROR_INVALIDDATA; + } else { + unsigned bpp = s->bpp/s->bppcount; + unsigned mul = 1 << (bpp - 8); + float max_val = (1 << bpp) - 1; + float coef[6]; + for (i = 0; i < 6; i++) { + unsigned num = ff_tget(&s->gb, TIFF_LONG, s->le); + unsigned den = ff_tget(&s->gb, TIFF_LONG, s->le); + if (!den) { + av_log(s->avctx, AV_LOG_ERROR, "ReferenceBlackWhite divisor is zero\n"); + return AVERROR_INVALIDDATA; + } + coef[i] = (float)num/den; + } + if (s->photometric == TIFF_PHOTOMETRIC_YCBCR) { + if (!coef[0] && coef[1] == max_val && coef[2] == (max_val + 1)/2 && coef[3] == max_val && coef[4] == coef[2] && coef[5] == coef[3]) + s->avctx->color_range = AVCOL_RANGE_JPEG; + // NOTE: TIFF 6.0 specification has an example where it mistakenly shows TV range coef[0] as 15 + else if (coef[0] == 16*mul && coef[1] == 235*mul && coef[2] == 128*mul && coef[3] == 240*mul && coef[4] == coef[2] && coef[5] == coef[3]) + s->avctx->color_range = AVCOL_RANGE_MPEG; + else { + av_log(s->avctx, AV_LOG_WARNING, "Unrecognized ReferenceBlackWhite values: [%g;%g] [%g;%g] [%g;%g]\n", coef[0], coef[1], coef[2], coef[3], coef[4], coef[5]); + s->avctx->color_range = AVCOL_RANGE_UNSPECIFIED; + } + } else { + if (!coef[0] && coef[1] == max_val && !coef[2] && coef[3] == max_val && !coef[4] && coef[5] == max_val) + s->avctx->color_range = AVCOL_RANGE_JPEG; + else if (coef[0] == 16*mul && coef[1] == 235*mul && coef[2] == coef[0] && coef[3] == coef[1] && coef[2] == coef[0] && coef[3] == coef[1]) + s->avctx->color_range = AVCOL_RANGE_MPEG; + else { + av_log(s->avctx, AV_LOG_WARNING, "Unrecognized ReferenceBlackWhite values: [%g;%g] [%g;%g] [%g;%g]\n", coef[0], coef[1], coef[2], coef[3], coef[4], coef[5]); + s->avctx->color_range = AVCOL_RANGE_UNSPECIFIED; + } + } + } + break; case TIFF_T4OPTIONS: if (s->compr == TIFF_G3) s->fax_opts = value; diff --git a/tests/ref/fate/exif-image-tiff b/tests/ref/fate/exif-image-tiff index 51580601e1..ebf2f38d6e 100644 --- a/tests/ref/fate/exif-image-tiff +++ b/tests/ref/fate/exif-image-tiff @@ -22,7 +22,7 @@ display_picture_number=0 interlaced_frame=0 top_field_first=0 repeat_pict=0 -color_range=unknown +color_range=pc color_space=unknown color_primaries=unknown color_transfer=unknown -- 2.13.2.windows.1