Is it a stream? Is it a file? ID3v2 header handling errors.
#1
I discovered an error when playing mp3 files served up via UPnP (HTTP). In short, large ID3v2 tags (like those with embeded album art) were causing problems syncing the MPEG stream and retrieving file format information in the codec. Details follow...

Some files played fine, while others would cause an exception when trying to output the audio. I initially assumed there was a UPnP problem as we are continually finding new ways in which the UPnP model refuses to fit nicely into the XBMC architecture, but I was wrong. It turns out that the real issue is the handling of 'Internet Streams' versus 'Files'. When we initialize the MP3 codec, we handle 'files' and 'streams' differently. For 'files' we use CMusicInfoTagLoaderMP3 to fetch some useful information about the bitrate and replay gain. A useful side-effect of this is that we end up parsing the ID3v2 tag (if there is one) and storing its size. We can then use this information later to skip the ID3v2 tag before decoding the MPEG data. For 'internet streams' we skip all of this (because we cannot seek in the stream) and assume that the data will consist only of MPEG frames. In most cases, this does not cause any real problems, as, luckily, the frame-sync logic in the codec keeps looking through the buffer until it finds a valid frame header. In the case of MP3 files with cover art stored in the ID3v2 tag (most of mine) the codec will not be able to sync, as our 'init' buffer is too small to contain the complete tag as well as a valid MPEG frame header. This means that the codec will not be able to retrieve the necessary bitrate, channel, and sample size information (boo), causing an error when we try to use junk values to initialize the audio output (whatever happens to be in memory).

The real problem here is that just because content is served up via a streaming protocol (as opposed to a random access method) it doesn't mean that we won't be given a complete file (metadata and all), as opposed to just a stream of encoded content (hence the title of the thread). I have implemented a fix/workaround (I haven't decided which I think it is) that remedies the problem with large ID3v2 tags (patch will be forthcoming), but I think this raises a larger question. Are we really thinking about what is being/can be served up via a particular method? The thing we really want to consider when handling input is whether or not the data can be accessed randomly or just sequentially. This changes the options available to us as far as passing off the parsing of tags, etc. This seems to be a recurring theme when it comes to HTTP sources, and I can think of other instances where it could be a problem. I am willing to look into re-writing a good bit of this logic, however, I am hesitant to undertake this if the core developers are not in favor, since the code would never be committed. What are your thoughts?

At any rate, I will be submitting a patch to fix the problem with embedded cover art. I would prefer to have the code in the MP3Codec::Init method, but it works most efficiently when placed in MP3Codec::Read. I am certainly open to suggestions about a better implementation.

Cheers
Reply
#2
Anything that improves the code will always be looked at favourably Smile

For streams it should certainly be able to handle skipping a suitably large amount of information to get a frame header. There possibly needs to be a limit on this, I'm not sure, but certainly it should be large enough to handle real-use scenarios.

Perhaps you could do a sketch up of what you have in mind for the broader changes, and we can then review the ideas and offer more specific comments?

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#3
For the ID3v2 tag, we can skip precisely the right amount of data by parsing the header. This 'should' hold true for most any header format, as it would otherwise be difficult to find the data stream.

For the broader changes, I am going to start with MP3Codec and then expand the scopr if that is successful. I will re-post when I have more to suggest.

The patch for the original bug has been submitted:
http://trac.xbmc.org/ticket/5858
Reply
#4
The patch has been committed to SVN. Thanks jmarshall.
Reply

Logout Mark Read Team Forum Stats Members Help
Is it a stream? Is it a file? ID3v2 header handling errors.0