[Scummvm-devel] Idea: IndexedMp3AudioStream

[Max Horn]

Am 08.10.2010 um 00:06 schrieb yotam barnoy:

> On Thu, Oct 7, 2010 at 9:48 PM, Max Horn <max at quendi.de> wrote:
>> 
>> This seek table can easily be created while computing the length of the MP3, but of course this is slow for big files (which is why this thread was started, after all ;). Still, the point I want to make is this: We could easily modify our MP3 stream class to support using a seek table. The code would be the same whether the seek table is created on the fly, or laoded from a precomputed cache file, or from an ID3 tag. Hence, I think it should be possible to add support for this in a relatively clean way.
> 
> As was discussed, I'm not sure how clean it would be in the case of a
> cache file, since our MP3 streams wrap around SeekableReadStreams.
> Finding a matching cache file would seem to require more extensive
> changes to our infrastructure.

Only if the cache is in a separate file; if it is embedded in the MP3 data, then obviously we don't need to make any changes to the infrastructure at all.

If we do want to use a separate cache file, though, the change would still be fairly small, and would be hidden almost completely inside mp3.cpp. 

The outside differences would be small:
We'd have to extend SeekableAudioStream::openStreamFile() to deal with this situation. A quick solution would be to add a special case for MP3; not elegant, but easy and effective. On the long run, we could design a more flexible system, but I think I'd prefer to do that generalization only when we encounter a second case where we actually need it... :)

Then, we would simply add a second factory method to mp3.h which takes a second stream (or maybe just a memory block) for the seektable cache file / data. In addition to driving the change in SeekableAudioStream::openStreamFile , that factor would also serve as a low-level interface engines could use if they wanted to support MP3 compressed game data files with a seektable embedded. 
However, I am not sure whether any engine really needs that, i.e. whether any engines has that big embedded MP3 "files".


> 
>> Now, as to how and where to store a seek table for an MP3 file..
>> 
>> It is true that ID3 v2.4.0 has a frame for a seek table, the ASPI "Audio seek point index" frame (see section 4.30 in <http://www.id3.org/id3v2.4.0-frames>). Unfortunately, according to <http://www.id3.org/Home> it is not really that wide spread, and clients may not support it properly (see <http://www.id3.org/Compliance_Issues>). Rather, it seems 2.3.0 is the most wide spread (and it does not cover the seek table data).
>> In v2.3.0, there is the MLLT "MPEG location lookup table" frame (see <http://www.id3.org/id3v2.3.0#head-febc618121880ee751b0c2a87327b0e698213dc4>), which also provides a seek table, but the format is mostly suitable for CBR files.
>> 
> 
> Correct me if I'm wrong, but according to what you're saying, we can't
> expect users to find rip tools that will help them create these
> things. Either we use ASPI which isn't supported, or MLLT which isn't
> what we need. Sounds like we'd have to build the tool on our own.

We should offer one, as part of our GUI tools: A tool which takes an MP3, scans it for the seektable, and if none is present, adds one. We should indeed do that regardless of whether there are rip tools out there which add this data, and regardless of whether we use XING, ID3 or whatever else. Simply because we want to give people an easy way to "upgrade" their existing MP3 files w/o having to re-rip them all, I'd say. With the GUI tool, it should be easy to do this in batch mode, too, i.e. convert all MP3s in a complete directory.

That said, I am pretty sure there are rip tools out there which can (or even by default) add this meta data; but not all do, so we'd have to come up with a list of tools that do, and with explanations on how to make them do this, etc.


> 
>> ID3 in either revision still has the advantage that it is at least a well-known unofficial "standard", and we would not have to create tools from scratch... But we would have to create the code for parsing this thing. Note that there is a sister library to libmad, called libid3tag; see also <http://www.id3.org/Implementations> for more implementations.
> 
> I would prefer not to include another whole library when we need only
> a small subset of the functionality. I think it's fine for making a
> tool that takes an mp3 and fills in the tag though.

Yeah; note that libid3tag also support creating ID3 frame data, so it would be useful for a tool that adds ID3 data to existing MP3s. For *parsing* those (esp. only parsing the few we are interested in, and skipping the rest), we could roll our own code. However, I am not sure how complex that parsing actually is. But since there are various open source libs for that out there, we can find out... ;)

However, the XING header might be a lot easier to parse (which is good: small code size, easier debugging :).

> 
>> Another method to record such a table inside an MP3 file is the XING MP3 header. I am not sure how much that is still used these days... But it is relatively simple to add this to an MP3 file, and used to be quite popular. See e.g. here for a description and sample code: <http://www.codeproject.com/KB/audio-video/mpegaudioinfo.aspx>.
> 
> Seems very similar to the ID3 tag, but it looks like mpegedit can
> automatically add the XING header, which solves a minor headache for
> us:
> http://mpgedit.org/mpgedit/man/mpgedit.html
> 
> 'mpgedit -X2 myfile.mp3' creates a new file with the Xing header. And
> it's available on Windows and Linux which is nice. I couldn't find any
> similar tool to add the VBR ID3 tags.

That sounds good then! (Note that there are tons of files which add ID3 tags, but I am not sure if any of these adds seek table data. Dang!).

> 
> I would have to say that right now Xing looks like the best option to
> me. The tool is already available, and the code to parse it is
> trivial. It's equivalent to the ASPI 8-bit version which is probably
> good enough (although I saw the 16-bit version was recommended for
> long files).
> 

Sounds great.

Cheers,
Max