On date Wednesday 2011-05-11 11:41:53 +0200, Kostya encoded:
> On Wed, May 11, 2011 at 11:29:04AM +0200, Stefano Sabatini wrote:
> > 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(-)
> [...]
> >
> > What happens if BitsPerSample or SamplesPerPixel are specified more
> > than once, or SamplesPerPixel is processed before BitsPerSample?
>
> TIFF specification says that entries should be sorted in ascending order by
> Tag, having multiple entries with the same ID is also probably wrong.
>
> > 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.
>
> Good approach, but in this case it's not needed since it's all ordered.
For example with 11.tiff this patch is working by chance, we have:
=> BitsPerSample = { 8 }
bpp = 8, bppcount = 1
init_image() doesn't fail just because the image is recognized as a
PAL8 image
=> SamplesPerPixel = 3
bpp = 24, bppcount = 3
init_image() correctly detects the right format
...
So I still consider the parse-all-and-init approach more robust (and
depending on less assumptions which may be broken by illegal
encoders).
--
By nature, men are nearly alike; by practice, they get to be wide apart.
-- Confucius
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
