On date Wednesday 2011-05-11 12:42:17 +0200, Kostya encoded: > 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?
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. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
