FS#10182 - Addition of librm to svn

Attached to Project: Rockbox
Opened by MohamedTarek (mtarek16) - Saturday, 02 May 2009, 09:14 GMT
Last edited by MichaelGiacomelli (saratoga) - Sunday, 13 September 2009, 23:52 GMT
Task Type Patches
Category Codecs
Status Closed
Assigned To No-one
Operating System Another
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


Addition of rm files to rockbox. It only adds the target files to rb's directory not to the actual build.
librm includes a makefile and a main.c for producing a test program that would convert an input rm file with cook codec to a wav file.
The code still needs a lot of cleaning.
   librm.patch (514.4 KiB)
This task depends upon

View Dependency Graph

This task blocks these from closing
 FS#6852 - Ability to play real player files 
Closed by  MichaelGiacomelli (saratoga)
Sunday, 13 September 2009, 23:52 GMT
Reason for closing:  Accepted
Additional comments about closing:  Accepted in r21695.
Comment by MohamedTarek (mtarek16) - Saturday, 02 May 2009, 09:20 GMT
Sorry, forgot to remove the libavutil/trash directory before making the previous patch
Comment by MichaelGiacomelli (saratoga) - Monday, 04 May 2009, 14:46 GMT
Good work! Next step is to clean up all the dead code in there.

I only skimmed quickly, but some impressions:

cook_float should probably be removed since we're not going to try and keep fp support.
The libavutil and ffmpeg_config.h defines you're actually using can probably be combined into a cook.h file or similar.
dsputill.h can probably be combined too.
dsputil.c probably doesn't have much in it beyond the windowing functions (ff_vector_fmul_add_add_c, etc), and these can be put into cook.c

Comment by MohamedTarek (mtarek16) - Wednesday, 06 May 2009, 15:25 GMT
Modified patch : Major code clean-up. I think it's almost ready to be ported.
   librm.patch (233.2 KiB)
Comment by MohamedTarek (mtarek16) - Thursday, 07 May 2009, 17:39 GMT
patch1 :
rm-wav converter using floating point cook decoder.

patch2 :
rm-wav converter using fixed point cook decoder.

patch3 :
clean code. (no ffmpeg-specific code)
Comment by MohamedTarek (mtarek16) - Saturday, 09 May 2009, 21:37 GMT
-Removed main() from cook.c to a separate main.c.
-Created cook.h which defines COOKContext and the decoder's functions protoypes.

Comment by Dave Chapman (linuxstb) - Saturday, 09 May 2009, 23:25 GMT
I've committed the last libcook.patch2 as r20897 and r20898 - thanks.
Comment by MohamedTarek (mtarek16) - Sunday, 10 May 2009, 21:27 GMT
Convert cook.c to use fixed-point arithmetic. Still didn't drop almost any floating point code, although some functions maybe (#if 0)'ed for cook.c to compile.
@Dave : Thanks for the commit.
Comment by Dave Chapman (linuxstb) - Sunday, 10 May 2009, 22:28 GMT
libcook.fixp_convert committed as r20901
Comment by MohamedTarek (mtarek16) - Sunday, 10 May 2009, 22:47 GMT
Drop floating point code from cook.c/h.
Remove fft.c, mdct.c and dsputil.h.
Comment by MohamedTarek (mtarek16) - Sunday, 10 May 2009, 22:59 GMT
Remove avcodec.h
Comment by MichaelGiacomelli (saratoga) - Sunday, 10 May 2009, 23:29 GMT
>Drop floating point code from cook.c/h.
>Remove fft.c, mdct.c and dsputil.h.

Committed in r20902.
Comment by Dave Chapman (linuxstb) - Monday, 11 May 2009, 09:02 GMT
libcook.del_avcodec committed as r20909
Comment by MohamedTarek (mtarek16) - Friday, 15 May 2009, 21:46 GMT
"Preliminary" patch for get_rm_metadata().
This patch enables Rockbox to recognize a rm/ra file and parse it to get the metadata for the WPS.
The only codec supported now is cook. However, cook codec in this patch is just a fake a codec (actually it's just the one I wrote
for my qualification task ) The purpose of the codec is just to make the file run for its length just to be able to check if the parsing
was done correctly.

For read_uint32be to work properly (to eliminate warnings during compiling ) I had to #undef uint32_t, and then redefine it to unsigned
int, as it seems it was defined for "long unsigned int". Not an elegant solution, but it's not my focus currently either.

Also I had to define read_uint8() [just reads a byte] and read_uint16be() as I couldn't find it in metadata_common.h. Should I add
a read_byte() and a read_uint16be() to metadata_common.h, or just define them whenever I need them ?
Comment by MohamedTarek (mtarek16) - Tuesday, 23 June 2009, 13:17 GMT
This is a quick-and-dirty patch for getting rm playback to work in the sim.
The patch isn't intended for committing, it's mainly for debugging purposes.
Comment by MohamedTarek (mtarek16) - Tuesday, 23 June 2009, 14:25 GMT
Sorry the patch is no good. I'm working on another one.
Comment by MohamedTarek (mtarek16) - Tuesday, 23 June 2009, 14:47 GMT
This one should work. I tried it on a clean source copy.
Comment by MohamedTarek (mtarek16) - Saturday, 04 July 2009, 09:44 GMT
This patch is to add support for rm playback in rockbox. Still no seeking.
Comment by MohamedTarek (mtarek16) - Sunday, 05 July 2009, 13:19 GMT
Fixed some issues from the previous patch (Pointed out by Dave Chapman on IRC) which are:
-TABs were mistakenly used as indentation in some files.
-svn:executable property was added for some files. (I really don't know how it was added !)
-Some local functions weren't declared static.
-No handling for big endian targets.

Note : I don't have a big endian target, so I'd appreciate it if someone who has one could test this patch on it.
Comment by MohamedTarek (mtarek16) - Sunday, 05 July 2009, 17:33 GMT
Still fixing for playback on big endian targets.
Comment by MohamedTarek (mtarek16) - Sunday, 05 July 2009, 22:21 GMT
Playback works on big endian targets. I have duplicated wma's bswap.h since it already optimized for rockbox.
Comment by MohamedTarek (mtarek16) - Sunday, 05 July 2009, 23:18 GMT
Some simple modifications on rm metadata parser.
Comment by Maurus Cuelenaere (mcuelenaere) - Monday, 06 July 2009, 08:57 GMT
I'm wondering why your read_uint*() implementations require pointers instead of just returning the data?

For example real_read_audio_stream_info() & rm_parse_header() could benefit of this, because the numerous variables could then be registers/on stack instead of memory variables (or am I wrong here and are these already on stack?)
Comment by MohamedTarek (mtarek16) - Wednesday, 08 July 2009, 13:55 GMT
The above patch was committed as r21695 after some modifications for reducing bin size.