[Ffmpeg-devel] [PATCH] Vorbis Encoder

[Oded Shimon]

On Sun, Oct 01, 2006 at 10:04:43AM +0200, Michael Niedermayer wrote:
> On Sat, Sep 30, 2006 at 11:33:49PM +0300, Oded Shimon wrote:
> > static int cb_lookup_vals(int lookup, int dimentions, int entries) {
> >     if (lookup == 1) {
> >         int tmp, i;
> >         for (tmp = 0; ; tmp++) {
> >                 int n = 1;
> >                 for (i = 0; i < dimentions; i++) n *= tmp;
> >                 if (n > entries) break;
> >         }
> 
> duplicate of nth_root() of vorbis.c

fixed

> this looks like a duplicate of vorbis_len2vlc() in vorbis.c

fixed

> that one looks like its part of vorbis_parse_setup_hdr_floors() from vorbis.c
> please remove all code duplications between vorbis.c & vorbis_enc.c
> ill review the rest after you removed all duplicate code

fixed

> why is this int and not (u)int8_t ?
> and the lines are too long

fixed

> >         { 1, 2,    -8.0,   1.0, 289, (int[]){ 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15, 0, 16, } },
> >     };
> 
> int -> uint8_t

fixed

> >     for (i = 0; i < rc->classifications; i++) {
> >         int a[10][8] = {
> >             { -1, -1, -1, -1, -1, -1, -1, -1, },
> >             { -1, -1, 16, -1, -1, -1, -1, -1, },
> >             { -1, -1, 17, -1, -1, -1, -1, -1, },
> >             { -1, -1, 18, -1, -1, -1, -1, -1, },
> >             { -1, -1, 19, -1, -1, -1, -1, -1, },
> >             { -1, -1, 20, -1, -1, -1, -1, -1, },
> >             { -1, -1, 21, -1, -1, -1, -1, -1, },
> >             { 22, 23, -1, -1, -1, -1, -1, -1, },
> >             { 24, 25, -1, -1, -1, -1, -1, -1, },
> >             { 26, 27, 28, -1, -1, -1, -1, -1, },
> >         };
> 
> int -> static const int8_t, some compilers will initalize such arrays
> every time the code gets executed ... 

fixed

> >     	int j;
> >     	for (j = 0; j < 8; j++) rc->books[i][j] = a[i][j];
> 
> memcpy()

fixed

> >     mc->magnitude = av_malloc(sizeof(int) * mc->coupling_steps);
> >     mc->angle = av_malloc(sizeof(int) * mc->coupling_steps);
> 
> av_malloc(sizeof(*mc->angle) ...
> 
> would be safer in case the type changes
> 
> 
> [...]
> >     int ady = FFMAX(dy, -dy);
> 
> ABS(dy)
> 
> 
> >     int base = dy / adx;
> >     int x = x0;
> >     int y = y0;
> >     int err = 0;
> >     int sy;
> >     if (dy < 0) sy = base - 1;
> >     else sy = base + 1;
> 
> id align these:
> if (dy < 0) sy = base - 1;
> else        sy = base + 1;
> 
> makes them look much nicer and more readable ...

this function now in vorbis.h, with those things fixed...


Try 2. vorbis_enc.c here..

- ods15
-------------- next part --------------
Index: libavcodec/allcodecs.c
===================================================================
--- libavcodec/allcodecs.c	(revision 6398)
+++ libavcodec/allcodecs.c	(working copy)
@@ -51,7 +51,7 @@
 #endif //CONFIG_MP3LAME_ENCODER
 #endif
 #ifdef CONFIG_LIBVORBIS
-#ifdef CONFIG_OGGVORBIS_ENCODER
+#if (defined CONFIG_OGGVORBIS_ENCODER && !defined CONFIG_VORBIS_ENCODER)
     register_avcodec(&oggvorbis_encoder);
 #endif //CONFIG_OGGVORBIS_ENCODER
 #if (defined CONFIG_OGGVORBIS_DECODER && !defined CONFIG_VORBIS_DECODER)
@@ -507,6 +507,9 @@
 #ifdef CONFIG_VORBIS_DECODER
     register_avcodec(&vorbis_decoder);
 #endif
+#ifdef CONFIG_VORBIS_ENCODER
+    register_avcodec(&vorbis_encoder);
+#endif
 #ifdef CONFIG_LIBGSM
     register_avcodec(&libgsm_decoder);
 #endif //CONFIG_LIBGSM
Index: libavcodec/Makefile
===================================================================
--- libavcodec/Makefile	(revision 6398)
+++ libavcodec/Makefile	(working copy)
@@ -122,6 +122,7 @@
 OBJS-$(CONFIG_VMDVIDEO_DECODER)        += vmdav.o
 OBJS-$(CONFIG_VMNC_DECODER)            += vmnc.o
 OBJS-$(CONFIG_VORBIS_DECODER)          += vorbis.o vorbis_data.o
+OBJS-$(CONFIG_VORBIS_ENCODER)          += vorbis_enc.o vorbis_data.o
 OBJS-$(CONFIG_VP3_DECODER)             += vp3.o
 OBJS-$(CONFIG_VP5_DECODER)             += vp5.o vp56.o vp56data.o
 OBJS-$(CONFIG_VP6_DECODER)             += vp6.o vp56.o vp56data.o
Index: libavcodec/avcodec.h
===================================================================
--- libavcodec/avcodec.h	(revision 6398)
+++ libavcodec/avcodec.h	(working copy)
@@ -2162,6 +2162,7 @@
 extern AVCodec vcr1_encoder;
 extern AVCodec ffv1_encoder;
 extern AVCodec snow_encoder;
+extern AVCodec vorbis_encoder;
 extern AVCodec mdec_encoder;
 extern AVCodec zlib_encoder;
 extern AVCodec sonic_encoder;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vorbis_enc.c
Type: text/x-csrc
Size: 57880 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20061001/5c6f07bb/attachment.c>