Many (minor) bugs in LibTheora 1.0b3, directory structure cleanup
I found many (minor) bugs (?) in LibTheora 1.0b3, and possibility (?) for optimization of the dir structure :
- Version string is not updated in INTERNAL.H
# define OC_VENDOR_STRING "Xiph.Org libTheora I 20071025 3 2 1"
I already had whined about this here: https://trac.xiph.org/ticket/1363
- Useless macro HIGHBITDUPPED defined 2x differently:
#define HIGHBITDUPPED(X) (((ogg_int16_t) X) >> 15)
#define HIGHBITDUPPED(X) (((signed short) X) >> 15)
In DCTDECODE.C and ENCODE.C .
In both files, it's used 1x only, so the IMHO solution is to delete it and replace with "direct" implementation using ogg_int16_t in both files. About not using OGG integer typdef's , YES, I had already whined about this here: https://trac.xiph.org/ticket/1329 "BITWISE.C typedef's in LibOGG" . So deleting some more silly INT's , SHORT's and LONG's at this occasion would be great.
- CPUID'ding is not completely barred out if "USE_ASM" isn't defined:
{{{void dsp_static_init(DspFunctions *funcs) { ogg_uint32_t cpuflags;
cpuflags = oc_cpu_flags_get (); dsp_init (funcs);
dsp_recon_init (funcs, cpuflags); dsp_dct_init (funcs, cpuflags); #if defined(USE_ASM) if (cpuflags & OC_CPU_X86_MMX) { dsp_mmx_init(funcs); }
ifndef WIN32
/* This is implemented for win32 yet */ if (cpuflags & OC_CPU_X86_MMXEXT) { dsp_mmxext_init(funcs); }
endif // WIN32
#endif // USE_ASM } }}}
Then IMHO "CPU.C" and "CPU.H" should get completely barred out from the compilaton, "oc_cpu_flags_get" not called, and "cpuflags" (correctly using "ogg_uint32_t" here :-) ) deleted also or set directly to 0 at least.
-
Directory structure: there are 2 dirs, "ENC" and "DEC" , plus some files elsewhere: "CPU." stuff above, some ".H" files in "INCLUDE" . Compiling the decoder doesn't require any files from "ENC", OTOH the encoder requires some files from "DEC" , nowhere documented what ones exactly, need to find out "the hard way" . Idea: create a directory "COMMON" and move the shared files into it.
-
"ENC" contains "ENCODER_DISABLED.C" ... useful ? Seems not to me ...
-
_ilog stuff is defined 2x differently:
static int _ilog(unsigned _v){
int ret;
for(ret=0;_v;ret++)_v>>=1;
return ret;
}
static int _ilog(unsigned int v){
int ret=0;
while(v){
ret++;
v>>=1;
}
return(ret);
}
IMHO 1 piece would be sufficient, using "ogg_uint32_t" for both "v" and "ret" , and a good candidate to be placed in "COMMON" , see 4. .
- BITWISE duplicate:
- Use a custom copy of the libogg bitpacker in the decoder
to avoid function call overhead.
Why not in the encoder also ? Idea: rename "BITWISE.C" and "BITWISE.H" of LibTheora into "TBITWISE.C" and "TBITWISE.H" to avoid confusion with "BITWISE.*" from LibOGG and delete dependency of LibTheora on *.C files of LibOGG. I had already whined about lack of typedef's, efficiency, compatibility and docs of BITWISE here: https://trac.xiph.org/ticket/1329 "BITWISE.C typedef's in LibOGG" .