On Wed, May 11, 2011 at 09:28:42AM +0200, Stefano Sabatini wrote:
> On date Wednesday 2011-05-11 07:39:07 +0200, Kostya encoded:
> > On Mon, May 09, 2011 at 12:51:24PM +0200, Stefano Sabatini wrote:
> > > Also add support for bits per component storage.
> > >
> > > Fix decoding of file 11.tiff, trac issue number #167.
> > >
> > > Based on a patch by Kostya Shishkov <[email protected]>.
> > > ---
> > > libavcodec/tiff.c | 138
> > > ++++++++++++++++++++++++++++-------------------------
> > > 1 files changed, 73 insertions(+), 65 deletions(-)
> >
> > Personally I don't understand that patch - why it assigns new meaning to
> > bpp,
> > why it saves all bits per component (I don't see it being used elsewhere)
> > and
> > what it does beside that.
>
> I'm trying to handle the cases:
>
> 1) BitsPerSample = { 8, 8, 8 }, SamplesPerPixel unspecifid
> If SamplesPerPixel is unspecified, the number of components is based on
> the number of components specified in the BitsPerSample tag.
> File b.tif, FFmpeg trac issue #168.
It's decoded the same (wrong) way with or without your patch, but has issues
if my old patch is applied indeed.
> 2) BitsPerSample = { 8 }, SamplesPerPixel = 3
> File 11.tiff, FFmpeg trac issue #167.
>
> In both cases totalbpp is computed with:
> s->totalbpp = 0;
> for (i = 0; i < s->bppcount; i++)
> s->totalbpp += s->bpp[i] ? s->bpp[i] : s->bpp[0];
>
> which issues the correct value in both cases.
Yes, you have a point, I just find your approach a bit strange.
See my attempt on it which does the same in a bit simplier way (IMO).
>From 71732ec8ae89a791c740fd173b5249dd84d8a9f5 Mon Sep 17 00:00:00 2001
From: Kostya Shishkov <[email protected]>
Date: Wed, 11 May 2011 10:27:57 +0200
Subject: [PATCH] make TIFF decoder handle samples per pixel tag
---
libavcodec/tiff.c | 97 ++++++++++++++++++++++++++++++++---------------------
1 files changed, 59 insertions(+), 38 deletions(-)
diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index 3cc3a42..3d40ec4 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -40,7 +40,7 @@ typedef struct TiffContext {
AVFrame picture;
int width, height;
- unsigned int bpp;
+ unsigned int bpp, bppcount;
int le;
int compr;
int invert;
@@ -210,6 +210,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;
+ 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, %d components)\n", s->bpp, s->bppcount);
+ 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 && 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)
{
@@ -263,6 +310,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;
@@ -282,46 +330,19 @@ 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);
+ if(init_image(s))
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");
+ break;
+ case TIFF_SAMPLES_PER_PIXEL:
+ if(count != 1){
+ av_log(s->avctx, AV_LOG_ERROR, "Samples per pixel is not a single value\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;
- }
+ if(s->bppcount == 1)
+ s->bpp *= value;
+ s->bppcount = value;
+ if(init_image(s))
+ return -1;
break;
case TIFF_COMPR:
s->compr = value;
--
1.7.0.4
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
