[FFmpeg-devel] [FFMPEG] [PATCH] cavs encoder

[Stefan Gehrer]

zhihang wang wrote:
> Hi, ffmpeg friends, I have added the CAVS encoder to ffmpeg. This is the
> corresponding patch file. Some variables on the CAVS decoder is also
> changed. You can use the following lines to test the cavs encoder.
> 
> Updated patch for today's SVN attached. A recommended procedure for
> trying it: Download http://www.hlevkin.com/TestVideo/foreman.yuv
> and encode it to AVS like this:
> 
> ./ffmpeg -vframes 50 -g 10 -psnr -bf 0 -s qcif -i foreman_qcif.yuv -vcodec
> cavs -b 1000 -refs 2 -strict -2 -subq 4 -cqp 16 -f rawvideo -bufsize 0
> out.avs
> 
> Also, you can change the GOP structure and the b frame number.
> 
> The patch is submitted for reviewing. I can make any changes if ffmpeg is
> needed to accept this patch.

Hi Wang,

thank you for working on this patch and providing it here.
I hope I will find some time soon to review it in more detail.

 From a quick look, I think the biggest problem with the patch
is the functional duplication between decoder and enoder.
It seems you copied many variables from the AVSContext to
the MpegEncContext and functions which already existed in
the decoder are duplicated, but now using the variables
from the MpegEncContext instead of the AVSContext.
I think it should be possible to have all relevant variables
in the AVSContext and use common functions in more places.
Whenever some variable from the MpegEncContext is needed,
for example options set by the user, one can use code
like

int foo(AVSContext *h) {
	MpegEncContext *s = &h->s;
	
	if(s->low_delay)
		...
}

to access those values.
That would also decrease the size of the patch and make
reviewing easier for others not so familiar with the code
base.

Best regards
Stefan