[Ffmpeg-devel] H.264 encoder

[Panagiotis Issaris]

Hi,

We've just put a new patch on our homepage which addresses
the remarks made.

Direct link to the patch:
http://research.edm.uhasselt.be/~h264/edm-20060622T170534-ffmpeg-h264.diff

Michael Niedermayer wrote:

>> >> ...
>> >> We felt that now was a good time to ask for some additional feedback.
>>     
> >
> > it contains a few pretty large tables which could maybe be generated at
> > runtime ...
>   

Fixed. Except for a table which contains statistically
gathered data on the relation between the SAE and the resulting
encoded size for a certain block. 


> > some tables are int while all values would fit in char
>   

Fixed.


> > some tables (ex: h264cavlc_encode_vlc_totalzeros_chromadc) have more
> > entries then ever used
> >
> > some tables based code could be replaced with tables-less (of coarse only
> > if its not slower)
> > for example:
> > +       static const int length_table[15][16] =
> > +       {
> > +               { 1,2,3,3 },
> > +               { 1,2,2 },
> > +               { 1,1 }
> > +       };
> > +
> > +       static const int code_table[15][16] =
> > +       {
> > +               { 1,1,1,0 },
> > +               { 1,1,0 },
> > +               { 1,0 }
> > +       };
> > +
> > +       put_bits(b,length_table[vlcnum][value],code_table[vlcnum][value]);
> >
> > with
> > if(vlcnum + value == 3) put_bits(b, value  , 0);
> > else                    put_bits(b, value+1, 1);
>   

We did a bit of benchmarking, and as in real usage the performance
difference seemed unnoticable, we used the proposed code.
We encoded various 1 min 560x304 size movies, which encoded
in about 32.5 seconds. The variations were in the range [32.2, 32.8]
and there seemed to be little correlation between the used code
and resulting encoding time.

When using the START_TIMER/STOP_TIMER code I got the following:

Original code using lookup table:
1075 dezicycles in h264calvc, 262099 runs, 45 skips

Lookup table using int8_t:
1054 dezicycles in h264calvc, 262102 runs, 42 skips

Suggested code:
1071 dezicycles in h264calvc, 262103 runs, 41 skips

We tried various other variations such as using a smaller lookuptable
combined with a conditional, but all resulted in about 107X "dezicycles".


> > some non table code would be much simpler with a table
> > for example:
> > +       switch(nc)
> > +       {
> > +       case 0:
> > +       case 1:
> > +               lookup_table = 0;
> > +               break;
> > +       case 2:
> > +       case 3:
> > +               lookup_table = 1;
> > +               break;
> > +       case 4:
> > +       case 5:
> > +       case 6:
> > +       case 7:
> > +               lookup_table = 2;
> > +               break;
> > +       default:
> > +               lookup_table = 3;
> > +       }
>   

Fixed.


> > then theres some do nothing code like:
> > +    case PIX_FMT_RGBA32:
> > +
> > +    case PIX_FMT_YUV444P:
> > +    case PIX_FMT_YUV422P:
> > +    case PIX_FMT_GRAY8:
> > +    case PIX_FMT_YUV411P:
> > +    case PIX_FMT_YUV410P:
> > +    default:
>   

Fixed.


> > theres mmx code in libavcodec instead of libavcodec/i386
>   

Fixed.


> > code duplication, ive not checked too carefully but loop filter & motion
> > compensation at least appear to be duplicated from the h264 decoder
>   

We didn't find a way to easily reuse the loop filter from the decoder, which
is why we wrote the routine ourselves. The motion compensation code in
libavcodec depends on the MpegEncoderContext, which we're not using. For this
reason, we wrote a simple motion compensation scheme ourselves, using the
optimized DspContext functions.


>> >> The patch is not intended for direct adoption into the main libavcodec
>> >> tree, but we'd be happy to make the required changes if the encoder is
>> >> considered adequate.
>>     
> >
> > for a encoder to be accepted in lavc it must at least:
> > * be clean & simple = no code where its immedeatly obvious how to do the
> >   same thing in half the number of lines
>   

Hopefully that's the case. We're open to suggestions though  :) 

> > * no redundant code
>   

Depends on whether using our own loop filter and motion estimation code is
considered redundant.


> > * processor specific code must be in the respective directories
>   

Fixed.


> > for this specific case i think further requireing that the encoder
> > must be at least one of the following 3 seems logical
> > * be simpler then x264
> > * produce better quality/bitrate then x264 at some fixed "speed"
> > * produce better quality/bitrate then x264 at the slowest speed of each
> > encoder
> >
> > [...]
>   

I think we would place our money on requirement one  :) 
I'm pretty sure our codec is simpler then x264, if only for the simple
reason
that it is _much_ smaller in codesize and number of features as we only
started working on it a few months ago  :) 

What specifically needs to be done now to get the patch integrated in
libavcodec?

With friendly regards,
Jori & Takis