[Ffmpeg-devel] tiff encoder (qualification task for GSoC)

[Justin Ruggles]

Bart?omiej Wo?owiec wrote:
> I send a solution of Qualification tasks "TIFF encode". I implemented 24bits 
> rgb tiff encoder. Of course, this solution is only to show my programming 
> abilities in C and knowledge about compression and ffmpeg project. I wrote 
> most parts of standard, which are included in decoder. In future I try to 
> develop more functions, especially including more parts of tiff standard.
> (more colors depths and more pictures in file).
> If you have any suggestions, please write.

Good job for a first draft.  As for extending it in the future, adding
TIFF's basic predictor is pretty simple and increases compression.
Also, TIFF supports YUV (it calls it YCrCb), which could be especially
useful for FFmpeg.

> [...]
> +	ptr = first;
> +	// write dirs entries
> +	while(ptr != end){
> +		tput_short(buf, ptr->tag, le);
> +		tput_short(buf, ptr->type, le);
> +		tput_long(buf, ptr->count, le);
> +
> +		buf_tmp = *buf;
> +		if(ptr->val == NULL || ptr->count == 1){
> +			switch(ptr->type){
> +			case TIFF_BYTE:
> +			case TIFF_SHORT:
> +			case TIFF_LONG:
> +				if(ptr->val){
> +					tnput(buf, 1, ptr->val, ptr->type, le);
> +				}else{
> +					tput(buf, ptr->off, ptr->type, le);
> +				}
> +				break;
> +			default:
> +				assert(0 && "not implemented");
> +			}
> +		}else{
> +			tput(buf, ptr->off, TIFF_LONG, le);
> +		}
> +		*buf = buf_tmp + 4;
> +		ptr++;
> +	}
> +	tput_long(buf, 0, le); // TODO offset to next picture
> +	return offset;
> +}

This is not quite correct.  What determines whether to use an offset or
write the values is whether the data is 4 bytes or less.  So both the
count and the data type determine this (up to 1 LONG, 2 SHORTS, or 4 BYTES).


> [...]
> +		default:
> +            av_log(s->avctx, AV_LOG_ERROR, "Chosen compression is not supported\n");
> +			return -1;
> +	}
> +	return 0;
> +}

It would be great to have JPEG compression here too.  There is a
technical note from Adobe on the correct way to do JPEG-in-TIFF.  LJPEG
is especially common, specifically for digital still cameras.  It might
be a bit complex trying to interface with the mjpeg encoder though.

> [...]
> +	if(avctx->pix_fmt != PIX_FMT_RGB24){
> +		av_log(s->avctx, AV_LOG_ERROR, "Only RGB24 is supported\n");
> +		return -1;
> +	}

grayscale and 1-bit would definitely be a nice addition. and not that
difficult really.

> [...]
> +	s->width = avctx->width;
> +	s->height= avctx->height;
> +	s->bpp = 24;
> +	s->le = 0; // choose endian
> +	le = s->le;

I agree with Michael here.  For decoding you need both byte orders, but
for encoding, just choose one and stick to it for simpilicity.

> [...]
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY, 0xfe, TIFF_LONG, 1, 0, NULL);
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY, TIFF_WIDTH, TIFF_LONG, 1, s->width, NULL);
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY, TIFF_HEIGHT, TIFF_LONG, 1, s->height, NULL);
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY, TIFF_BPP, TIFF_SHORT, 3, 0, bpp_tab);  
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY, TIFF_COMPR, TIFF_SHORT, 1, s->compr, NULL);
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY, TIFF_INVERT, TIFF_SHORT, 1, 2, NULL); 
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY, TIFF_STRIP_OFFS, TIFF_LONG, strips, 0, strip_offsets); 
> +
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY,0x115, TIFF_SHORT, 1, 3, NULL);  // TIFF_SAMPLES_PER_PIXEL
> +
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY,TIFF_ROWSPERSTRIP, TIFF_LONG, 1, s->rps, NULL); 
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY,TIFF_STRIP_SIZE, TIFF_LONG, strips, 0, strip_sizes); 
> +#define SOFTWARE_NAME "FFmpeg"	
> +	add_entry(&dir, dir_buf + TIFF_MAX_ENTRY,0x131, TIFF_STRING, strlen(SOFTWARE_NAME), 0, SOFTWARE_NAME);

You should change 0xfe, 0x115, and 0x131 to something like
TIFF_NEWSUBFILETYPE, TIFF_SAMPLESPERPIXEL, and TIFF_SOFTWARE.

Also, the count in ASCII types (Software in this case) must include the
trailing NULL char.

It might be cleaner to always pass the count and a pointer instead of
passing a count, a value, and a pointer.  Then the function can just use
the type and count to determine whether to write the values directly in
the entry or in an offset.

1 spec nitpick.  All TIFF files must, technically, have the XResolution,
YResolution, and ResolutionUnit tags.  I think it's a silly requirement,
but I didn't write the spec.

-Justin