On Wed, May 11, 2011 at 12:13:40PM +0200, Stefano Sabatini wrote:
> 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
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?
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
