[Ffmpeg-devel] H.264 encoder

[Rich Felker]

On Fri, Jun 23, 2006 at 07:00:36PM +0200, Michael Niedermayer wrote:
> > +#ifdef H264_DEBUG_WRITE_DECODED_IMAGE
> > +static void ff_h264_append_image(uint8_t *data, AVCodecContext *avctx)
> > +{
> > +	int f = open("/tmp/teststream.yuv",O_CREAT|O_WRONLY|O_APPEND,S_IRUSR|S_IWUSR);
> > +	write(f,data,avctx->width*avctx->height*3/2);
> > +	close(f);
> 
> write() and close() is not ok in a codec

Indeed.

> > +	if (chromaDCcount == 0)
> > +	{
> > +		if (chromaACcount == 0)
> > +			CodedBlockPatternChroma = 0;
> > +		else
> > +			CodedBlockPatternChroma = 2;
> > +	}
> > +	else
> > +	{
> > +		if (chromaACcount == 0)
> > +			CodedBlockPatternChroma = 1;
> > +		else
> > +			CodedBlockPatternChroma = 2;
> > +	}
> 
> if(chromaACcount) CodedBlockPatternChroma= 2;
> else              CodedBlockPatternChroma= !!chromaDCcount;

Yes this removes a conditional in one of the two cases, and it's much
more readable.

> [...]
> > +	if (CodedBlockPatternLuma > 0)
> > +	{
> > +		for (j = 0 ; j < 4 ; j++)
> > +		{
> > +			int X = (j%2)*2;
> > +			int Y = (j/2)*2;
> 
> % and / in more or less inner loops are not ok, especially when this is
> just (j&1)*2 and j&2

Note that the compiler cannot optimize this unless it's smart enough
to figure out that j is always nonnegative.

> [...]
> > +	for (y = 0 ; !done && y < 4 ; y++) for (x = 0 ; !done && x < 4 ; x++) if (residual->part4x4Y[0][0][y][x] != 0) done = 1;
> > +	for (y = 0 ; !done && y < 4 ; y++) for (x = 0 ; !done && x < 4 ; x++) if (residual->part4x4Y[0][1][y][x] != 0) done = 1;
> > +	for (y = 0 ; !done && y < 4 ; y++) for (x = 0 ; !done && x < 4 ; x++) if (residual->part4x4Y[1][0][y][x] != 0) done = 1;
> > +	for (y = 0 ; !done && y < 4 ; y++) for (x = 0 ; !done && x < 4 ; x++) if (residual->part4x4Y[1][1][y][x] != 0) done = 1;
> 
> find a new keyboard your enter key is broken

lol :)

> > +
> > +/**
> > + * Can contain pointers to the relevant starting points in a picture
> > + */
> > +typedef struct _MacroBlock
> 
> stuff begining with _ is reserved in C ...

Only if the second character is uppercase or another _, but here it is
uppercase so yes it's reserved.

Rich