Re: [MP3 ENCODER] AIFC - MP3 encoding
[...] So I'm back to square 1, how can I go from AIFC to MP3? I have no real experience with sound file formats and I really just want to write a front end which uses tools written by people who know a lot more than me such as lame :) Any tips are appreciated. There's one really simple solution to this, and that is to either compile in libsndfile ( http://www.zip.com.au/~erikd/libsndfile/ ) with LAME, or get a precompiled one (check out the links at the project page: http://www.mp3dev.org/mp3/ ) which uses libsndfile... - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] Parameter setting functions...
Just thought I'd say my thoughts on the different parameter setting function proposals we've had so far... Individual functions for each parameter: Pros: - None. ;) Cons: - Litters the API with "thousands" of functions. - If the parameter's type changes, the API has to change. - If a new parameter is introduced, the API has to change. Giving a parameter structure: Pros: - Hmmm, none. ;) Cons: - You have to be very careful not to disturb the order of the parameters. - You end up with a bunch of duplicates if you have to change parameters. - Different compilers can cause different alignments. Giving tag-pairs on stack to one function which parses them: Pros: - API never has to change. Cons: - Littering with different tags for each type. - It's possible to pass the wrong type. Giving tag-pairs on stack to 3 functions (one for each type): Pros: - API never has to change. - One tag for any type. Cons: - It's still possible to pass the wrong type, but it's much clearer since the function itself states which to pass. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
IIRC, if you use varargs then the compiler can't detect if the supplied parameters are of wrong type. Example : [...] The compiler won't notice that the parameter is of wrong type and when the function will try to extract an int from the stack, it will get garbage. This is indeed true, and that's why I chose to divide the function in 3 to clarify this to the user. But if this is of no concern , then a simpler interface can be used : [...] LAME_END_MARKER informs the lame_set_parameters to stop reading values, as AFAIK This is exactly how my current interface works (based on TagItems when passed on stack). there is not other way to determine whether all the args has been read the __FLOAT and __INT attachments are there only to help illustrate my idea, but could be helpful in the actual implementation too Yes, but this still requires the parameters to be correct, and parsing gets abit more messy... - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
o main.c can't hold lame_global_flags, change lame_init() to allocate and return a pointer to lame_global_flags. o Move lame_global_flags out of lame.h, it should never be accessed externally. I think we have to keep the old interface, since several applications use it. So lame_global_flags will still have to be instantiated by the calling program and exposed to the calling program. I dont think this is incompatiable with a shared library? Yes, it is, because the structure itself can expand and change at any given time internally, and if it's allocated and/or changed externally it will most surely get corrupted by any program using a different version of the structure than the library itself... - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
and parsing gets abit more messy...
How ?
[...]
This is how it looks now, better than using va_arg() at each case imho...
void lame_set_int(lame_global_flags *gfp, lame_param_tag tag1, ...)
{
va_list ap;
int param; /* (varargs promotes integrals to int) */
va_start(ap, tag1);
while (tag1 != TAG_END)
{
param = va_arg(ap, int);
switch (tag1)
{
case LAMEtag1:
gfp-test1 = param;
break;
case LAMEtag2:
gfp-test2 = param;
break;
default:
break;
}
tag1 = va_arg(ap, lame_param_tag);
}
va_end(ap);
}
I can still implement a way to have the tag also contain the type of the
parameter, but since the general consensus seemed to be that this would be
too complicated I went for this approach... ;)
- CISC
--
MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
This is how it looks now, better than using va_arg() at each case imho... Why is it better ? You wasted an additional variable and gained ( maybe ) some code space by not doing a function call ( or maybe it is a macro , probably platform dependant ) every time. It's usually a (rather messy) macro. ;) By writing almost the same code 3 times you increase the likehood of bugs 3 times :-) Alltough most of the code would be repeated anyway. True, but there's not much space for error. ;) [...] #define CASE(TAG,elem,type) case LAME_ ## TAG : gfp - elem = va_arg(args, type); break; CASE( SET_SAMPLING_FREQ , samp_freq, int ) [...] That's actually not a bad idea... /* in my man page va_start has only one argument (ap) ... */ Your man-page is wrong then I think, the second argument is so that va_arg() knows where to start (ie, it makes ap point to the next argument). I can still implement a way to have the tag also contain the type of the parameter, but since the general consensus seemed to be that this would be too complicated I went for this approach... ;) But your existing tag "contain" the type , at least as much as they would in my method. What I meant was that I could device another way of passing the tags that would allow me to supply information of the parameters type (ptr/float/int), but this isn't too convenient at the calling end. #ifdef YOU #define SOME_TAG some_int_value #else #define SOME_TAG some_int_value #end What was the purpose of this? Unless I use SOME_TAG__WARNING_THIS_IS_FLOAT_AND_NOT_INT_OR_ANYTHING_ELSE, but that is not the point , since it would work both with your and my code. The advantage of having 3 different functions is also that you at a later stage can change the type of the internal parameter, and still accept the old without having to add a duplicate tag (f.ex. if you change from int to float, then you can just add the tag to lame_set_float(), and add a cast in lame_set_int)... - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
This sounds more complex than my proposed "void*" change. Keep in mind: More complex, but not that hard. "KISS". I'm not even gonna ask. ;) "Keep It Simple, Stupid". [...] Right, well, I believe that's exactly what it is now. ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
This way there is no need to parse any strings, we don't pass any pointers, the setup routine would just be a big switch/case. This is basically the way TagItems work when passed on stack, in fact, that combined with another identical function that does float we're pretty much set. Why do we need float at this point ?? Because several of the parsed arguments are floats? Well, anyway, I just finished up the following: /* Set a parameter with string-pointer */ void lame_set_string(lame_global_flags *gfp, lame_param_tag tag1, ...); /* Set a parameter with float or double value (varargs promotes floats to double) */ void lame_set_float(lame_global_flags *gfp, lame_param_tag tag1, ...); /* Set a parameter with char, short or int value (varargs promotes integrals to int) */ void lame_set_int(lame_global_flags *gfp, lame_param_tag tag1, ...); This can cater for any kind of parameter, and the functions themselves are very simple, yet can take several parameters in a row... Should I start implementing this now, or wait after 3.87 release? Mark? What needs to be changed to make a real shared library: o Exchange all direct access to lame_global_flags in parse.c with calls to lame_set_xxx(). o main.c can't hold lame_global_flags, change lame_init() to allocate and return a pointer to lame_global_flags. o Move lame_global_flags out of lame.h, it should never be accessed externally. o ..and any other changes still needed for re-entrance ofcos. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
Why do we need float at this point ?? Because several of the parsed arguments are floats? ie. frequencies could be passed in Hertz as ints, was just something to think about now while we change the API anyway Yes, but no reason not to have the possibility there. ;) Should I start implementing this now, or wait after 3.87 release? I think 3.87 is already out, CVS is tagged 3.88 alpha 1 Ah, yes, so I see... You may get in contact with Takehiro to see what will be done next. Takehiro, wanna drop me a line when you are finished moving stuff about? ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
1.) Floats and doubles will lose all it's precision (this can be simply avoided by passing the string-pointer from arg-parsing and converting when parsing TagItems instead) What about passing the pointer to the float/double instead? Sure, but that means you will have to temporarily store the value somewhere else. Yes. Are there many fixed values in lame? If you read parse.c you'll see that there's a few values that's atof()ed straight in. simply making a new TagItems interface which allows you to specify the type of data the TagItem contains. This will be much better in the long run, and will allow the TagItems to contain any type of data. This sounds more complex than my proposed "void*" change. Keep in mind: More complex, but not that hard. "KISS". I'm not even gonna ask. ;) The first proposal is IMHO enough for everyday use, the second is for architectures which didn't allow lossless transportation of information between a void* and an integer type (in case you use ti_data to contain the information instead of a pointer to the information). Only "problem" I can see immediately with void* is that all values needs to be casted, which wasn't strictly necessary with unsigned long, and float/ double still needs to be temporarily stored. :P I'm waiting for your solution which didn't need casts (and remember the mail which talked about casts in lame). Eheh, well, we'll see what I can cook up. ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
[...] This way there is no need to parse any strings, we don't pass any pointers, the setup routine would just be a big switch/case. This is basically the way TagItems work when passed on stack, in fact, that combined with another identical function that does float we're pretty much set. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
Why not something like: lame_set_float(char *variable_name, float val); lame_set_int(char *variable_name, int val); Hey, gr8, idea, and maybe another with void*? But this brings up another question: The code is going to do a lot of parsing, and it looks like each variable will require several lines of code? If that is the case, then we dont actually save anything and we might as well just creating a short function for every variable? This way there are no type casting problems. Ah, no, I just got a gr8 idea, what we need is 3 functions: lame_set_string(lame_global_flags *gfp, StringItem tag1, ...); lame_set_float(lame_global_flags *gfp, FloatItem tag1, ...); lame_set_int(lame_global_flags *gfp, IntItem tag1, ...); Dead simple parsing using 3 different TagItem clones, and you can pass several items at once (within the same type anyway). - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] TagItem issues...
1.) Floats and doubles will lose all it's precision (this can be simply avoided by passing the string-pointer from arg-parsing and converting when parsing TagItems instead) What about passing the pointer to the float/double instead? Sure, but that means you will have to temporarily store the value somewhere else. 3.) 64bit pointers will be truncated (not too much trouble unless you have and use more than 4gig of memory in your machine). Don't keep the AmigaOS compatibility of my functions (see bellow why) and change the data part (ti_Data) of the struct to "void*". Hmmm, yes, maybe, if the compiler allows it. ;) simply making a new TagItems interface which allows you to specify the type of data the TagItem contains. This will be much better in the long run, and will allow the TagItems to contain any type of data. This sounds more complex than my proposed "void*" change. Keep in mind: More complex, but not that hard. the Amiga TagItem implementation was done with 32bit pointers in mind and not with portability. An (IMHO) more elegant (and still simple) solution for the data part would look like [...] The first proposal is IMHO enough for everyday use, the second is for architectures which didn't allow lossless transportation of information between a void* and an integer type (in case you use ti_data to contain the information instead of a pointer to the information). Only "problem" I can see immediately with void* is that all values needs to be casted, which wasn't strictly necessary with unsigned long, and float/ double still needs to be temporarily stored. :P - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] TagItem issues...
Ok, after thinking about this for a while, and experimenting a little, I thought of a couple of issues with using TagItems to pass configuration settings in LAME... 1.) Floats and doubles will lose all it's precision (this can be simply avoided by passing the string-pointer from arg-parsing and converting when parsing TagItems instead) 2.) Passing stringpointers instead of values is all well and good with shell interface, but might be abit inconvenient in other (future) interfaces. 3.) 64bit pointers will be truncated (not too much trouble unless you have and use more than 4gig of memory in your machine). I've been thinking about simply ditching AmigaOS TagItem compatability, and simply making a new TagItems interface which allows you to specify the type of data the TagItem contains. This will be much better in the long run, and will allow the TagItems to contain any type of data. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] LAME as a shared lib
or in an Amiga TagItem like way (I have an implementation (~2k source, most of it are comments) of struct TagItem and NextTagItem() if you need it), less difficult to use (you didn't need a parser). And how about volunteering to add this to LAME? :-) NextTagItem() (no problem, .gz or .bz2?) or lame_setup_stuff() (sorry, not enough spare time at the moment)? Actually, I wouldn't mind doing this, as I already have CVS access... ;) (either .gz or .bz2 would do fine) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] LAME as a shared lib
A much better (and tidier) solution is to have one function that you can pass a struct to, like this: lame_setup_stuff(struct LAMEprefs *prefs, unsigned int structsize); 1) use commandline like strings to setup LAME's encoding engine handle = lame_get_handle(); lame_setup_stuff(handle, "output samplerate = 32 kHz"); lame_setup_stuff(handle, "input samplerate = 48 kHz"); lame_setup_stuff(handle, "bitrate = 128 kps"); or in an Amiga TagItem like way (I have an implementation (~2k source, most of it are comments) of struct TagItem and NextTagItem() if you need it), less difficult to use (you didn't need a parser). Actually, that is an excellent idea, and much more flexible than a struct, can't think of why I didn't think of that... ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] LAME as a shared lib
The fix is to do what Frank suggests: go to a data-encapsulted type interface, where every parameter must be set via a function. We would need to write about 100 functions along the lines of: lame_set_input_samplerate(). A much better (and tidier) solution is to have one function that you can pass a struct to, like this: lame_setup_stuff(struct LAMEprefs *prefs, unsigned int structsize); And then simply just don't read more than the supplied size, and be sure that when you extend this struct you just append new variables at the end, and never change the order or size of the variables (if size must change, make a new variable at the end)... Perhaps also a version-number of the struct as the first item to make it easier to decide what options to read, and which to discard... - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] gcc option for TAKEHIRO_IEEE754 hack
I just made a new TAKEHIRO_IEEE754_HACK more ANSI C compliant. I think we can make it default for Intel architecture. This seems to work fine for PPC now (giving a speedboost of 10-15%), but 68k still doesn't work (tons of reservoir errors), IIRC, 68k doesn't use IEEE754 format at all - Motorola used a floating-point format of their own. Yes, internally they use an 80bit extended format, but I was under the impression that the normal 64bit double was standard, but I haven't checked, anyone know this for sure? PPC seems to be IEEE754 though. ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] Conformed all this-is-included-defines...
I just conformed all this-is-included-defines (f.ex. UTIL_H_INCLUDED) to the 'project_file_name' style (f.ex. LAME_UTIL_H), this gives a good unique name that shouldn't clash with any other defines (internal or system) already existing... I know it might seem rash touching so many files at once, but it was needed since there were so many different styles going in the different files that it was all a big mess .. looks much better now... BTW, I left mpglib untouched since it's based on another project, and we don't want to mess too much with it for future compatability (although, is it still in development?), if Mark finds it appropriate, he can change it... ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] Ogg Vorbis beta 2 still has a long way to go.
Reading Roel's initial statement, and all the replies, I'd like to comment a little on this matter myself... Yes, Roel was abit rash, but more than just a little justified imho. Don't deny that quite a few of you were overpraising Vorbis long before it actually was much to praise .. sure, it's a format with much potential, but it hasn't reached much of that potential yet, and stating that it's an "mp3-killer" already is way exagerated. I'm not off trying to start a flamewar or anything, but I just want you all to stop and think for a while, and consider your own actions, and please cease all future defamatory remarks... Offnote: Please find a way to avoid using alloca() which Vorbis seems to use *alot*, it's not very portable, and is a horrible stack-abuse. ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] changes to version.c
#include "sndfile.h" should be added to version.c on or around line 25, sf_get_lib_version is undefined otherwise. Of course it only makes a difference if you use libsndfile, but it caused me a problem. Actually, I just fixed that, and the correct solution is to include get_audio.h which in turn includes sndfile.h .. the reason for this is that there's a special alignment for WIN32 apparently. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] mpglib compiles once again...
Right, just committed a bunch of (quick and ugly) fixes to mpglib so it should once again compile without spewing errors .. it's probably still completely broken though, I hadn't time to test. Someone should take some time going over mpglib and clean it up abit (Frank, since you broke it, why don't you fix it?) Also possibly fixed a mono-decoding bug in layer1.c?! - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] Please stop breaking LAME... ;)
:: I run gcc 2.95.(3) and SAS/C (the latter is usually much more helpful on :: warnings, and still much more forgiving on "errors")... Can you add g++ for testing? Or also some other C++ compiler? Or use gcc at monday/wednesday and friday and g++ at the rest? I have no incentive to use C++ at all, infact I'm not even interested... :: Ofcos, as I said, the best thing is to use the correct one to begin with, :: but if that isn't possible, atleast cast the value into the correct one. Write a (inline) function to convert from type A to type B. Wild type casting in C is one of the most dangerous things. I'm not talking about "wild" typecasting, I'm saying that strict, controlled typecasting can be good, instead of leaving it up to the compiler to make the choices (which might not always be the correct one)... C can't cast from type A to type B. There is only the possibility to cast every shit to type B. And this is dangerous. So you write a int ifreq; double dfreq; ifreq = (int) ( dfreq ); and now dfreq changes the type to 'struct bla*'. C converts this without batting an eyelid. What the hell are you on about?! Oh, w8, you mean if some smartass changes dfreq to a struct without checking the code, or atleast changing the name so these kinds of conflicts can occur? I'd say he'd be downright stupid. ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] Please stop breaking LAME... ;)
I hope you guys correct all the errors. Please make sure that no new bugs show up on the next beta release! If it's very much "broken" I think it'll be best to start again with a previous alpha, one before someone broke it. Thank you all who are working hard on making LAME such a great encoder! Well, when I checked in the last fixes this morning, everything compiled and ran fine (finally), now there are more changes again, so we'll have to wait and see... ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] Please stop breaking LAME... ;)
Are you sure about that? The Layer2 decoding code doesn't compile for me, and when I try to decode it says something about bug!!?? or something like that, every file I try to decode does it (mp3, I disabled layer1 and layer2 for now just so it would compile). Ah, yes, I hadn't come that far yet (I usually use my own amiga_mpega.c interface)... :P - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] Please stop breaking LAME... ;)
:: Please check that your code compiles and works ok before checking it in :: to the CVS ... :: That doesn't help fully to solve the problem. Compiler and runtime libs are different. So the check can only be done for one (or two) compilers. Still, it's not fun when certain files fail to compile because of silly errors (like conflicting prototypes). ;) Currently my aim is, that the program is compilable with: gcc 2.95.2 g++ 2.95.2 I run gcc 2.95.(3) and SAS/C (the latter is usually much more helpful on warnings, and still much more forgiving on "errors")... :: Also it would be nice if you cast types that don't match the functions :: types into the correct one Casting is a bad thing. Correct the reason for the casting, not the warning (or the error in C++) itself. See Ada Rationale. It is Ada related, but the reasons are the same for every procedural language like C, Pascal, Ada, ... Ofcos, as I said, the best thing is to use the correct one to begin with, but if that isn't possible, atleast cast the value into the correct one. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] LAME ID3 version 2 support proposal
Why add version 2 tags at all? Version 1 tags are virtually useless within streaming audio (since they don't appear until the end of the stream), so version 2 tag support is essential for that type of application. Not really. Take a look at my streaming MP3 station on Live365 at http://www.live365.com/cgi-bin/directory.cgi?autostart=wtgfs and you'll see the info from the ID3 version 1 tags for all clips. The server, which of course runs much faster than the internet connection, simply reads the ID3 Version 1 tag and uses the info before it starts streaming the music. A non-problem, really. Well, he's not talking about broadcast streaming, but streaming directly from f.ex. a webserver .. in this scenario it's quite useful that the first piece of info you get is the ID3v2 tag... Besides, ID3v2 offers alot more than ID3v1 can and ever will be offering, so imho adding ID3v2 support is a great idea... - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] ..more on the PPC AmigaOS bug...
This is a bug in the tables - that first line should be a 0 3
instead of 0 2. I have just fixed this, but it should have very
little effect on the output.
Ok, nice that we found that one then. ;))
And I dont think this could cause any crashes though. Can you
try adding a check in that loop to make sure
b gfc-npart_s
Sounds to me like the bo_s[] array is probably getting corrupted
somewhere else?
Sadly, I think this is an optimizer bug actually, if I add somthing like
if (b=CBANDS) printf("%d\n",b);
in that loop, everything works just fine. :P
Now this is really sad since the SAS/C compiler is no longer maintained. :/
..but didn't someone else (Kimmo Mustonen?) say that he tried with gcc, and
that that gave the same problems?!
- CISC
--
MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] lame-3.83beta
Did you try to increase the stack size? LAME 3.8x allocates local stack space for the GTK variables even if you don't use GTK (some #ifdef are gone) and this might lead to your problems. I've set the stack size of 512 kB and LAME works rock steady on my PPC Mac... Hmm... that's something that I haven't thought of carefully enough. I have used the stack size of 1 meg and that should be enough and I know that m68k side uses that amount of stack but I really don't know whether the startup code for PPC programs uses that stack size or something else (a predefined value, maybe?)... and I don't know how to easily change the stack size on PPC and AmigaOS. I think there are some PPC Amiga programmers here as well so I'm interested in knowing how to do that. Usually the compiler (SAS/C at least) on m68k side can automatically check and/or increase the stack size on the fly, if required and configured. But I don't know if this works on PPC side or not. Anyone who knows better? There is no easy way to change the stack size in the code... The only way is to set the shell-stack before running the program .. oh, and btw, LAME has never used more than 300k here, and 100k is usually safe... - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] lame-3.83beta
For some reason the latest versions of lame have crashed on my system when running on PPC processor. The same code compiled for m68k processor doesn't crash. I think the problem is probably memory access at invalid address, but because of the system design, on m68k it luckily doesn't seem to cause a problem but on a PPC side it really does. Yep, I too have been noticing this lately (and is the reason I haven't released any binaries lately)... :P [...] Any idea? Does this kind of problem occur in any other architecture? If the problem can't be found, at least inserting some assertions could be a good idea. I can continue debugging more if needed. This problem seems to be linked with optimization, as I just tried a compile without optimization (vbrquantize.c fails with some weird error, compile that *with* optimization (don't ask me why)), and without optimization things seem to work... - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] Fixed silly mp3input bug with libsndfile...
Right, just committed a fix for a silly little bug which prevented mp3input to work together with libsndfile... I also noted that the --decode feature doesn't care about endianess, thus ending up with a big endian wav (read; garbage. ;) ) on big endian machines. Couldn't think up a quick fix for this due to the way this feature works, though it could be nice if it used libsndfile (and thus being able to write almost any type of file) when available... ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] Weird stuff...
Ok, someone just broke something... ;) ..just tried the latest CVS stuff, and with Takehiro's IEEE hack weird stuff happen with -h switch; not only is 72 bytes missing in the final file, but the end gets trashed .. if I examine the file under a hexeditor, I can see that several frames at the end are padded, starting with only a few bytes, then more and more, this is obviously totally wrong! ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] MPEG 2.5
"LAME --resample 8 -b 8 input output" (MPEG2.5) - Notice how everything is garbled. just add a "--noshort -mm" here The sample I tested with was already mono .. and if --noshort is needed in MPEG2.5, why isn't it default? there is no need for --noshort in general with MPEG2.5, but this combination of 8 kHz and 8 kbits seems to require it. maybe there are simply not enough bits for short blocks. Well, I just tested, and it still sounds garbled at 8kHz .. however if I upsample to f.ex. 11kHz, it sounds much better (although not perfect (16kHz sounds better, which I find weird. ;) )) mpeg2.5 support is relatively new in Lame and there is some more fine tuning necessary. All your feedback is welcome ;) Yep, and afaics it's slightly broken. ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] MPEG 2.5
Just thought I'd note that MPEG 2.5 is more or less broken at lower bitrates, and that the filters seem screwy at low frequencies... Try the following: (preferably with a 8kHz sample) "LAME --resample 8 -b 8 input output" (MPEG2.5) - Notice how everything is garbled. "LAME --resample 16 -b 8 input output" (MPEG2) - Notice how this sounds just fine compared to the previous. Now, find a 8kHz sample somewhere and try this: "LAME -b 8 input output" (Resamples to 16kHz (fix it when 2.5 works)) - Notice how soft the output sounds. "LAME -k -b 8 input output" - Notice the vast difference. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] hack for IEEE754 FPU
This feature is not default because it is deeply depending on IEEE754 floating point calculation and there're some computers NOT compatible to IEEE754. (but it is so rare :p) What exactly does the IEEE754 consist of, and what part does your hack exploit? ;) I am interested in the result of this hack, on various plathome. pls check and tell me the result and your plathome(CPU and compiler). I just did another test with the latest CVS, and -h seems to work now (and noticeably faster than the version without the hack without options), however without options there's alot of scratchy noises in the output, but a whopping 50% speed-increase!!! Like someone mentioned earlier, the 68k has 80bit math internally, but afaik is only capable of returning 64bit .. if this affects the hack, I dunno, perhaps you do? ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] hack for IEEE754 FPU
The binaries runs fine, and "make test" result is OK, too. but the speed is slower than the no hacked version *sigh* S It's faster here, but as mentioned of very poor quality. :/ BTW, is it the same result, with or without -h option ? -h gives a much better mp3, but there's still blurbs and noise in it. :P - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] free format bitstreams and iso documentation
[...] As Gaby pointed out, it says the "decoder is not requires to support..." which doesnt say they are invalid MP3 files :-) Hey, btw, I just tested free-format with mpega.library and AmigaAMP's internal decoder (FhG licensed), and both failed miserably. :/ - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] hack for IEEE754 FPU
This feature is not default because it is deeply depending on IEEE754 floating point calculation and there're some computers NOT compatible to IEEE754. (but it is so rare :p) When trying this hack the results got extremely noisy and of very poor quality. This was on 68k, I've been unable to test on PPC (read below). The compiler also gave me several "type punning" warnings, this is probably intentional, however it also gave me some warnings about integer operations overflowing, this perhaps is not so good... I am interested in the result of this hack, on various plathome. pls check and tell me the result and your plathome(CPU and compiler). I tested on AmigaOS (68k, SAS/C compiler). [!Developers read please!] On another note I'd like to mention that there seems to be something that trashes memory badly in the current CVS version, as I'm unable to run a PPC compiled version of LAME (coredumps immediatly with access violations), 68k version seems to work (it's not as sensitive as the PPC kernel), but causes weird lockups after a while... However, it seems to work fine in VBR mode (-v -V4), so the culprit is probably not being used there, or atleast inflict less damage. I am currently underequipped to do much debugging myself, so I hope some of you that I know are much better at this sort of thing can. ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] NOPOW define added in CVS...
I just committed a NOPOW define to CVS which when enabled will replace certain pow() combinations with optimized variants .. this gives an average of 20% speedincrease on my machines (68k PPC), might be more or less on yours, try it out... ;) It made no difference on my pentium II. Ok, it seems 68k (040/060) benefits the most from this, because it doesn't have exp/log functions in the FPU, and has to emulate them... - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] NOPOW define added in CVS...
S I just committed a NOPOW define to CVS which when enabled will S replace certain pow() combinations with optimized variants .. S this gives an average of 20% speedincrease on my machines (68k S PPC), might be more or less on yours, try it out... ;) Why don't you use table :) all pow(2.0, global_gain*0.25) and pow(2.0, -global_gain*0.1875) could be rewrite table lookup. Yes, that's probably alot more sensible... ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] 8bit patch for get_audio.c (w/libsndfile)
Included is a quick patch to allow you to encode from 8bit files (wav/8svx etc) when using libsndfile... Oh, btw, how about automatic resampling when the input samplerate doesn't match any of the allowed mpeg samplerates? ;) I just applied the patch. Good idea about the resampling. ..it's worth noting the patch is a temporary thing until libsndfile correctly converts the 8bit data to proper 16bit... I just had to add a few lines to lame.c: if the output samplrate is not valid, it will downsample to the nearest valid rate. Does it also upsample if it's below 16kHz? ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] 8bit patch for get_audio.c (w/libsndfile)
Included is a quick patch to allow you to encode from 8bit files (wav/8svx etc)
when using libsndfile...
Oh, btw, how about automatic resampling when the input samplerate doesn't match
any of the allowed mpeg samplerates? ;)
*** get_audio.c Tue Nov 16 21:43:30 1999
--- get_audio.c.new Sun Nov 21 23:05:49 1999
***
*** 12,17
--- 12,18
static int samp_freq;
static int input_bitrate;
static int num_channels;
+ static int bitwidth;
int read_samples_pcm( short sample_buffer[2304],int frame_size, int samples_to_read);
int read_samples_mp3(FILE *musicin,short int mpg123pcm[2][1152],int num_chan);
***
*** 374,379
--- 375,381
printf("sections :%d\n",gs_wfInfo.sections);
printf("seekable :\n",gs_wfInfo.seekable);
#endif
+ bitwidth=gs_wfInfo.pcmbitwidth;
}
}
***
*** 404,409
--- 406,414
if (samples_read0) samples_read=0;
for (; samples_read frame_size; sample_buffer[samples_read++] = 0);
}
+
+ if (bitwidth==8) for (; samples_read 0; sample_buffer[samples_read] =
+sample_buffer[samples_read--] * 256);
+
return(rcode);
}
- CISC
--
MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] 8bit patch for get_audio.c (w/libsndfile)
+ if (bitwidth==8) for (; samples_read 0; sample_buffer[samples_read] = sample_buffer[samples_read--] * 256); Whoops, that should ofcos be "samples_read = 0;"... :P - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] Sorry...
Sorry I ever mentioned that __buffsize thing, it's a SAS/C specific feature, and shouldn't be in LAME .. if Amiga-porters like to use it, I suggest they go to http://csc.smsu.edu/~strauser/audio.html fetch the source available there, and look at my amiga_version.c which automatically generates an Amiga version-string, and sets a pre-defined buffersize for 68k/PPC. So, please remove the following which has been added to main.c in 3.56: #ifdef OS_AMIGAOS /*#include memory.h*/ extern int __buffsize = 100;/* performance increase of about 10% */ #endif /* OS_AMIGAOS */ - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] Something is wrong again. :P
Something is wrong again, as I now am experiencing random crashes again, this has been long gone, but somewhere around 3.30-3.50 it has been reintroduced. :( I'm not sure, but I believe the previous problem was due to buffer overflow or some dodgy memory allocation .. if anyone could check this out, it'd be gr8. ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] more patch merging... mdct fft
check this new patch containing new MDCT/subband filtering and FFT.
This patch can be applied to both of the original LAME 3.37beta(3.50stable)
and LAME3.37 with my yesterday Huffman coding patch.
Check mdct_sub in your newmdct.c...
void mdct_sub(
short *w0, short *w1,
double (*mdct_freq)[2][576], int stereo,
III_side_info_t *l3_side,
int mode_gr)
{
int gr, k, ch;
double *mdct_enc = mdct_freq[0][0];
double work[36];
short *wk[2] = {w0, w1};
This last declaration isn't valid ANSI, and I believe it's a GCCism as
someone mentioned earlier in this list... ;)
- CISC
--
MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] buffer
is there a buffer option in Lame? I need larger IO bufs. Heh, as a matter of fact, I've just started a little discussion about that. ;) If you are getting LAME from The Amiga Alternative Audio Page, the next version there will probably have a bigger static buffer even if there haven't been an official buffering implementation by then. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] lame 3.36beta
Kimmo Mustonen: Much code cleanup and Amiga updates Hummm, any chance of telling me which changes he did? I'd be very interested. ;) Why's that? Well, not all my patches were included, but some of them were Because I'm doing a semi-official port of LAME for Amiga... ;) Check out The Amiga Alternative Audio Page - http://csc.smsu.edu/~strauser/audio.html [...] Oh, btw, I've been hinted by one of my (LAME port) users that putting f.ex; extern int __buffsize=10; That's something that I'm really interested. I noticed myself that there are far too many unnecessary context switches for PPC version to be efficient. I must try if even 1 meg or something would give even more performance. Yes, but I think it might be wiser setting different sizes for input/output for efficient streaming using the setvbuf() function. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] lame 3.36beta
Oh, btw, I've been hinted by one of my (LAME port) users that putting f.ex; extern int __buffsize=10; now what kind of importable, dubious hack is _this_?!!! It's no hack, it's pure ANSI .. it's the reference buffer value for input/output. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] File IO buffering.
Concerning the file IO buffering discussed earlier, I'd suggest a scheme somewhat resembling the following so as to scalably buffer at a rate that is actually considering what amount of data we input/output. Using setvbuf() we set buffers thusly; Input: (samplerate * 2) * number_of_channels Output: (bitrate / 8) * 1024 This should imho make an ideal setting for buffering, both for streaming and normal operation... ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] File IO buffering.
Concerning the file IO buffering discussed earlier, I'd suggest a scheme somewhat resembling the following so as to scalably buffer at a rate that is actually considering what amount of data we input/output. Using setvbuf() we set buffers thusly; Hummm, ofcos, when reading up on how setvbuf() works I see that it is not recommended that one uses it after the file has been used. Doing it this way (which I still think we should) would then mean one would have to close the file after getting all the info needed from it's headers, reopen it, set the buffers, then skip to the beginning of the data. ..a little extra work, but I think it would be preferable to a fixed size buffer. - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] lame 3.36beta
Kimmo Mustonen: Much code cleanup and Amiga updates
Hummm, any chance of telling me which changes he did?
I'd be very interested. ;)
More stdin fixes: For some reason, forward fseek()'s would fail when
used on pipes even though it is okay with redirection from "". So I
changed all the forward fseek()'s to use fread(). This should improve
stdin support for wav/aiff files. If you know the input file is raw
pcm, you can still use the '-r' option to avoid *all* seeking of any
kind.
At line 353;
int fskip(FILE *sf,long num_bytes,int dummy)
{
char data[num_bytes];
..this doesn't work (atleast not here), as the char data[..] needs a static
value .. I have no idea how to do this a better way though... :/
Oh, btw, I've been hinted by one of my (LAME port) users that putting f.ex;
extern int __buffsize=10;
somewhere, f.ex. in main.c would allocate about 100k as a buffer for all
fread/fwrite etc operations .. I tested this, and it gave me about a 5% speed
increase in general.
- CISC
--
MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] LAME 3.34 fixes...
There's a few problems in 3.34 that needs to be fixed... ;)
1. In get_audio.c the new wav/aiff routines presume that u_short etc are
pre-typedef'ed without even including sys/types.h.
2. In l3bitstream.c line 90 the following is done;
if ( frameResults == NULL )
{
frameResults = (BF_FrameData*) calloc( 1, sizeof(*frameData) );
assert( frameData );
}
this must be wrong (a quick cut'n'paste job? ;) ), I presume;
if ( frameResults == NULL )
{
frameResults = (BF_FrameResults*) calloc( 1, sizeof(*frameResults) );
assert( frameResults );
}
is the correct way. ;)
3. No biggie, but;
masking_lower = pow(10,masking_lower_db/10);
at line 655 in quantize.c should prolly be;
masking_lower = pow(10.0,masking_lower_db/10.0);
to prevent unecessary conversions...
4. Also #define inline should prolly be a Makefile define (instead of adding
all kinds of systems at line 20 in takehiro.c), as more systems than MSV_C
use a different inline code...
- CISC
--
MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] Time status oddities...
..just noticed something weird with the timestatus that seems to occur when encoding MPEG2-layer3... It seems to report being twice as fast as it really is, this also seems to only occur on stereo samples, so might it be that it adds both channels into the timeline instead of treating them as one?! This was tested with Mathew Hendry's latest time-status stuff, but I seem to recall that it did the same before... - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
[MP3 ENCODER] Something weird...
There seems to be something weird about how LAME encodes audio here when I compare output from 68k and PPC versions of the program... The MP3's differ quite a lot in normal mode, but not so much in -f mode, this is however not an audible difference afaich, and comparing the decoded output again shows that the samples often just are off by 1 step or so... The MP3's looks like parts of the data sometimes gets shifted 1 character, so I'm guessing there is some mis-setting/assumption of a datatype or something somewhere, if wanted, I can create two small test-mp3s that can be examined by someone who might have a clue... ;) - CISC -- MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] A few suggestions for code improvement...
Ok, first off, a lot of functions are left empty with no prototype
declared, like f.ex. "void lame_init()", instead of doing it properly like
"void lame_init(void)" .. doing this will eliminate about half of the
compiler warnings (and also declaring prototypes in the appropriate .h
file)...
..then there are lots and lots of dead assignments that probably are left
over from old code and never got removed...
send me a patch :-)
Ok, I'll go through all those warnings and fix them and send you a patch... ;)
In get_audio.c at line 369 (and 210) "if ((".." (samples_read = 0))" ..
what is the purpose of this, as samples_read is a unsigned long, and thus
can never be less than zero?!
This should probably be considered a bug, since samples_read is supposed
to be able to handle a -1 return value. I changed it to an int.
Heheh, ok, I thought that looked weird. ;)
- CISC
--
MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
Re: [MP3 ENCODER] A few suggestions for code improvement...
In get_audio.c at line 369 (and 210) "if ((".." (samples_read = 0))" ..
what is the purpose of this, as samples_read is a unsigned long, and thus
can never be less than zero?!
This should probably be considered a bug, since samples_read is supposed
to be able to handle a -1 return value. I changed it to an int.
Hmmm, I think you broke it .. you forgot a few things... ;)
1. Line 15: "DWORD getaudio(...)" .. ie, it still returns unsigned long (as
typedef'ed in common.h). ;)
2. Line 179 and 321: read_samples() returns int, but everything inside uses
unsigned long. ;)
- CISC
--
MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
