On date Wednesday 2011-05-11 10:38:22 +0200, Kostya encoded:
> 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.
Yes there is another issue, I have another patch for that.
> > 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;
What happens if BitsPerSample or SamplesPerPixel are specified more
than once, or SamplesPerPixel is processed before BitsPerSample?
That's why I prefer to init_image() just after all the tags have been
read, and bits-per-pixel-components and samples-per-pixel information
has been already collected.
--
Backward conditioning:
Putting saliva in a dog's mouth in an attempt to make a bell ring.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
