On date Wednesday 2011-05-11 13:22:07 +0200, Kostya encoded:
> On Wed, May 11, 2011 at 01:10:19PM +0200, Stefano Sabatini wrote:
[...]
> > > You're right, and having init_image() after all those tags are parsed is
> > > better (I cannot argue with that), but I still think it's better to
> > > operate
> > > with sum of pixels per component from the beginning, so the flow would be:
> > >
> > > => BitsPerSample = { 8 }
> > > => SamplesPerPixel = 3
> > > bpp = 8, bppcount = 1, spp = 3
> > >
> > > since bppcount == 1 multiply bpp by spp, so final bpp = 8 * 3 = 24
> > >
> > > => BitsPerSample = { 8, 8, 8 }
> > > => no SamplesPerPixel or = 3
> > >
> > > bpp = 24, bppcount = 3, spp = 1 (default setting) or 3
> > > since bppcount != 1, ignore spp (optionally check if bppcount != 1 && spp
> > > !=
> > > bppcount and complain then)
> > > final bpp is unchanged and is 24
> > >
> > > What do you think?
> >
> > Still doesn't work in the illegal case when we have more than one
> > SamplesPerPixel tags, but that's not a very likely case, so if you
> > prefer this approach I'm fine with it.
>
> Probably it's better to implement generic checking for unsorted and/or
> repeating tags anyway by keeping track of last tag id (and if it's unsorted
> then it's incorrect format no matter how many identical tags are in that
> file).
Even in this case it would be best practice to issue a warning and try
to decode the file properly anyway, indeed there is nothing
intrinsically wrong in unsorted or duplicated tags, even if they're
not allowed by the specs.
Patch updated according to this logic, but I want to note that I
consider the previous approach more robust and thus preferable
(doesn't assume tags unicity and a specific order, avoids
SamplesPerPixel/BitsPerSample coupling and the need for custom
checks).
Anyway the new patch is simpler, and the other changes can be easily
applied on top of it, so I'm fine with this patch.
>From 76d7ee6b0511016bff90b17d919b2b78518e741c Mon Sep 17 00:00:00 2001
From: Stefano Sabatini <[email protected]>
Date: Mon, 9 May 2011 12:51:24 +0200
Subject: [PATCH] tiff: add support to TIFF_SAMPLES_PER_PIXEL case in tiff_decode_tag()
Format detection and internal frame initialization is moved to a
separate init_image() function, which is called when all the tags have
been read, and so both BitsPerSample and SamplesPerPixel information
has been collected.
Fix decoding of file 11.tiff, trac issue number #167.
Based on a patch by Kostya Shishkov <[email protected]>.
---
libavcodec/tiff.c | 118 ++++++++++++++++++++++++++++-------------------------
1 files changed, 62 insertions(+), 56 deletions(-)
diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index f252913..63e3a7d 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -38,7 +38,7 @@ typedef struct TiffContext {
AVFrame picture;
int width, height;
- unsigned int bpp;
+ unsigned int bpp, bppcount;
int le;
enum TiffCompr compr;
int invert;
@@ -208,6 +208,53 @@ static int tiff_unpack_strip(TiffContext *s, uint8_t* dst, int stride, const uin
return 0;
}
+static int init_image(TiffContext *s)
+{
+ int i, ret;
+ uint32_t *pal;
+
+ switch (s->bpp*10 + s->bppcount) {
+ case 11:
+ s->avctx->pix_fmt = PIX_FMT_MONOBLACK;
+ break;
+ case 81:
+ s->avctx->pix_fmt = PIX_FMT_PAL8;
+ break;
+ case 243:
+ s->avctx->pix_fmt = PIX_FMT_RGB24;
+ break;
+ case 161:
+ s->avctx->pix_fmt = PIX_FMT_GRAY16BE;
+ break;
+ case 324:
+ s->avctx->pix_fmt = PIX_FMT_RGBA;
+ break;
+ case 483:
+ s->avctx->pix_fmt = s->le ? PIX_FMT_RGB48LE : PIX_FMT_RGB48BE;
+ break;
+ default:
+ av_log(s->avctx, AV_LOG_ERROR, "This format is not supported (bpp=%d, bppcount=%d)\n", s->bpp, s->bppcount);
+ return AVERROR_INVALIDDATA;
+ }
+ if (s->width != s->avctx->width || s->height != s->avctx->height) {
+ if ((ret = av_image_check_size(s->width, s->height, 0, s->avctx)) < 0)
+ return ret;
+ avcodec_set_dimensions(s->avctx, s->width, s->height);
+ }
+ if (s->picture.data[0])
+ s->avctx->release_buffer(s->avctx, &s->picture);
+ if ((ret = s->avctx->get_buffer(s->avctx, &s->picture)) < 0) {
+ av_log(s->avctx, AV_LOG_ERROR, "get_buffer() failed\n");
+ return ret;
+ }
+ if (s->bpp == 8 && s->picture.data[1]){
+ /* make default grayscale pal */
+ pal = (uint32_t *) s->picture.data[1];
+ for (i = 0; i < 256; i++)
+ pal[i] = i * 0x010101;
+ }
+ return 0;
+}
static int tiff_decode_tag(TiffContext *s, const uint8_t *start, const uint8_t *buf, const uint8_t *end_buf)
{
@@ -261,6 +308,7 @@ static int tiff_decode_tag(TiffContext *s, const uint8_t *start, const uint8_t *
s->height = value;
break;
case TIFF_BPP:
+ s->bppcount = count;
if(count > 4){
av_log(s->avctx, AV_LOG_ERROR, "This format is not supported (bpp=%d, %d components)\n", s->bpp, count);
return -1;
@@ -280,46 +328,16 @@ static int tiff_decode_tag(TiffContext *s, const uint8_t *start, const uint8_t *
s->bpp = -1;
}
}
- switch(s->bpp*10 + count){
- case 11:
- s->avctx->pix_fmt = PIX_FMT_MONOBLACK;
- break;
- case 81:
- s->avctx->pix_fmt = PIX_FMT_PAL8;
- break;
- case 243:
- s->avctx->pix_fmt = PIX_FMT_RGB24;
- break;
- case 161:
- s->avctx->pix_fmt = PIX_FMT_GRAY16BE;
- break;
- case 324:
- s->avctx->pix_fmt = PIX_FMT_RGBA;
- break;
- case 483:
- s->avctx->pix_fmt = s->le ? PIX_FMT_RGB48LE : PIX_FMT_RGB48BE;
- break;
- default:
- av_log(s->avctx, AV_LOG_ERROR, "This format is not supported (bpp=%d, %d components)\n", s->bpp, count);
- return -1;
- }
- if(s->width != s->avctx->width || s->height != s->avctx->height){
- if(av_image_check_size(s->width, s->height, 0, s->avctx))
- return -1;
- avcodec_set_dimensions(s->avctx, s->width, s->height);
- }
- if(s->picture.data[0])
- s->avctx->release_buffer(s->avctx, &s->picture);
- if(s->avctx->get_buffer(s->avctx, &s->picture) < 0){
- av_log(s->avctx, AV_LOG_ERROR, "get_buffer() failed\n");
- return -1;
- }
- if(s->bpp == 8){
- /* make default grayscale pal */
- pal = (uint32_t *) s->picture.data[1];
- for(i = 0; i < 256; i++)
- pal[i] = i * 0x010101;
- }
+ break;
+ case TIFF_SAMPLES_PER_PIXEL:
+ if (count != 1) {
+ av_log(s->avctx, AV_LOG_ERROR,
+ "Samples per pixel requires a single value, many provided\n");
+ return AVERROR_INVALIDDATA;
+ }
+ if (s->bppcount == 1)
+ s->bpp *= value;
+ s->bppcount = value;
break;
case TIFF_COMPR:
s->compr = value;
@@ -462,7 +480,7 @@ static int decode_frame(AVCodecContext *avctx,
AVFrame *picture = data;
AVFrame * const p= (AVFrame*)&s->picture;
const uint8_t *orig_buf = buf, *end_buf = buf + buf_size;
- int id, le, off;
+ int id, le, off, ret;
int i, j, entries;
int stride, soff, ssize;
uint8_t *dst;
@@ -503,21 +521,9 @@ static int decode_frame(AVCodecContext *avctx,
return -1;
}
/* now we have the data and may start decoding */
- if(!p->data[0]){
- s->bpp = 1;
- avctx->pix_fmt = PIX_FMT_MONOBLACK;
- if(s->width != s->avctx->width || s->height != s->avctx->height){
- if(av_image_check_size(s->width, s->height, 0, s->avctx))
- return -1;
- avcodec_set_dimensions(s->avctx, s->width, s->height);
- }
- if(s->picture.data[0])
- s->avctx->release_buffer(s->avctx, &s->picture);
- if(s->avctx->get_buffer(s->avctx, &s->picture) < 0){
- av_log(s->avctx, AV_LOG_ERROR, "get_buffer() failed\n");
- return -1;
- }
- }
+ if ((ret = init_image(s)) < 0)
+ return ret;
+
if(s->strips == 1 && !s->stripsize){
av_log(avctx, AV_LOG_WARNING, "Image data size missing\n");
s->stripsize = buf_size - s->stripoff;
--
1.7.2.3
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
