[FFmpeg-devel] [PATCH] adding xavs encoding support
Diego Biurrun
diego
Sun Jul 18 10:04:03 CEST 2010
On Sun, Jul 18, 2010 at 02:47:46PM +0800, jianwen chen wrote:
> On Sat, Jul 17, 2010 at 6:55 PM, Diego Biurrun <diego at biurrun.de> wrote:
>
> > > --- libavcodec/libxavs.c (revision 0)
> > > +++ libavcodec/libxavs.c (revision 0)
> > +#include "avcodec.h"
> > > +#include <xavs.h>
> > > +#include <math.h>
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <string.h>
> > > +#define END_OF_STREAM 0x001
> >
> > Place system headers before local headers.
>
> OK, however, I also refer to the libx264.c with the same order for the
> including headers.
That file does it wrong then.
> I can change the order as following
>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <math.h>
>
> > +#include "avcodec.h"
> > +#include <xavs.h>
Better, but only slightly, you still place xavs.h after avcodec.h.
> --- libavcodec/libxavs.c (revision 0)
> +++ libavcodec/libxavs.c (revision 0)
> @@ -0,0 +1,355 @@
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <math.h>
> +
> +#include "avcodec.h"
> +#include <xavs.h>
Place all system headers before local headers.
> +/* Analyze i8x8 (requires 8x8 transform) */
> +#define XAVS_PART_I8X8 0x002
> +/* Analyze p16x8, p8x16 and p8x8 */
> +#define XAVS_PART_P8X8 0x010
> +/* Analyze b16x8, b*/
> +#define XAVS_PART_B8X8 0x100
more readable:
#define XAVS_PART_I8X8 0x002 /* Analyze i8x8 (requires 8x8 transform) */
#define XAVS_PART_P8X8 0x010 /* Analyze p16x8, p8x16 and p8x8 */
#define XAVS_PART_B8X8 0x100 /* Analyze b16x8, b */
> + for (i = 0; i < nnal; i++) {
> + /* Don't put the SEI in extradata. */
> + if (skip_sei && nals[i].i_type == NAL_SEI) {
> + x4->sei = av_malloc( 5 + nals[i].i_payload * 4 / 3 );
> + if(xavs_nal_encode(x4->sei, &x4->sei_size, 1, nals + i) < 0)
if (
> + if( !bufsize && !frame && !(x4->end_of_stream) ){
Inconsistent formatting; you again miss the space after the if and put
spaces inside the parentheses.
> + /*Amanda thinks there is no IDR frame in AVS 1.0.Sequence header is used as a flag*/
long line
> + x4->params.pf_log = XAVS_log;
> + x4->params.p_log_private = avctx;
> +// x4->params.i_frame_total = max_frames[CODEC_TYPE_VIDEO];
Why is this commented out but still present?
> + /*AMANDA TAG: AVS doesn't allow B picture as reference and the max allowed number of B is 2*/
see above
And what is AMANDA?
> + /*AMANDA TAG: Amanda thinks this is only used for counting the fps*/
Here and in other places: space after /* and before */ would aid
readability.
> + if (avctx->me_method == ME_EPZS){
> + x4->params.analyse.i_me_method = XAVS_ME_DIA;
> + }
> + else if (avctx->me_method == ME_HEX){
> + x4->params.analyse.i_me_method = XAVS_ME_HEX;
> + }
> + else if (avctx->me_method == ME_UMH){
> + x4->params.analyse.i_me_method = XAVS_ME_UMH;
> + }
> + else if (avctx->me_method == ME_FULL){
> + x4->params.analyse.i_me_method = XAVS_ME_ESA;
> + }
> + else if (avctx->me_method == ME_TESA){
> + x4->params.analyse.i_me_method = XAVS_ME_TESA;
> + }
> + else{
> + x4->params.analyse.i_me_method = XAVS_ME_HEX;
> + }
This could be a switch IMO.
Diego
More information about the ffmpeg-devel
mailing list