On Sat, 2012-10-13 at 00:35 +0200, Kevin Chabowski wrote: > I am currently trying to implement downloading tracks to HiMD for > QHiMDTransfer, I have some questions regarding this: Thanks you very much for trying to implement it. > Is there a way to tell if a file is actually an MP3 file or not, using > libmad (could not find anything in their "documentation", perhaps someone > here knows better)? This is a technical problem insofar as there is no real MP3 file header at all. A file that contains MP3 music consists of concatenated MPEG audio data frames, and those indeed do have a header. There is no need for the MP3 file to start with a valid MP3 frame (and indeed, if the file is tagged with an ID3v2 tag, it does not do so), nor there is a need with the last MP3 frame ending at the end of the file. (and again, if an APE and/or ID3v1 tag is contained in that file, it does not do so). > I looked at the code of himdcli and could not find a > file type checking, so I just tried to transfer something else and it > "succeeded". The error handling of himdcli currently in fact does not exceed the works-for-me level of quality. If the current position of the stream does not contain a valid MPEG audio frame header, mad_header_decode does return an error code. This might be fine at the beginning of the file, as having ID3v2 tags there is quite common. An error is also quite common at the end of the file, as there might be an MP3 tag. As MPEG audio is designed, it is mostly self-synchronizing, i.e. even if garbage data is read from the stream, or some part of the stream is missing, the decoder is able to find the next header with high reliability by just guessing. So "loosing synchronization", as libmad calls it, i.e. having no header at an expected position, is a *recoverable* error. This kind of error is generally ignored. What you end up by trying to upload a non-MP3-file is getting the parts of that file that remotely look like valid MPEG data frames uploaded to the player, and all other (i.e. most) parts of that file skipped. You can easily find out the mess when you dump the mp3 track using himdcli again. Of course you are not the first person to have this request. A quick google query resulted for example in this thread: http://www.mars.org/pipermail/mad-dev/2004-April/001021.html The basic idea of MP3 file validation in my oppinion is to try to sync on the first frame (for which one might use code that is able to skip well-known tags, but you don't have to), and then check that you don't lose synchronisation for a certain number of frames. Be sure to not consider a single call of mad_header_decode that succeeds to indicate synchronisation - the header found might be spurious. At least require two succeeding frames to be successfully read before concluding that there the decoder locked. > If there is no way, should I just check, if the file ends with '.mp3'? > Would not be a really good solution, but better than nothing IMHO. In the GUI, I would not consider to offer the user to upload any file not having a supported extension, i.e. only MP3 at the time being. A basic MP3 sanity check as discussed above would be a good idea. > I would need to recycle some code from himdcli. Simply copying and pasting > is obviously a terrible solution. Should I add higher level functions to > libhimd (seems to be low-level orientated) or introduce a new library for > that? As long as you keep to plain basic C, as it is used in libhimd (and himdcli), please feel free to move the code to libhimd. It is what should be done in my oppinion anyway. The code might need some clean-up before, though. As we have an himd_mp3stream, we should add an himd_mp3writestream or possibly add functions to write mp3 streams using the same himd_mp3stream object structure, if sensible. For a first attempt at getting the stuff work, you could even move the whole himd_writemp3 function into libhimd (and declaring the utility functiosn of it static) and leave cleanup for a later time. You probably are right that this function really is one layer too high for libhimd. The abstraction layer libhimd currently provides is on the layer of streams and frames, so opening the file, extracting ID3 tags and splitting it into frames should not really go into libhimd, but generating the TOC entry for the track index (including adding the strings to the string table) would belong there. I do not want to put the burden of redesigning the interface onto you, though, so doing the GUI part first, and just moving the stuff from himdcli is the right start. > If the latter, someone else would need to integrate it into these > strange Qt Project files. I have no idea at all how these work... No problems, we can help you with that. Regards, Michael Karcher