Hello, For non-mmap'ed input implementations, I have encountered a situation where due to a truncated last frame, an infinite loop occurs.

Where mad_header_decode() returning MAD_ERROR_BUFLEN is interpreted as "need more input", my input caching routine continues to return the unconsumed partial frame. mad_header_decode() says that's not enough and the process just goes on.

The complication I have here is my input routine is a random access input cache routine and doesn't know an EOF has truly occurred until input data is requested beyond the end of file. Thus seeking N times to a partial last frame (or any other frame) could happen.

One ugly hack I have found to avoid this is to increase the 8 byte zero pad sufficiently to satisfy mad_header_decode()'s internal buffer length validation. Whereupon an attempt is made to seek beyond the input length and an EOF can be detected. Another approach is to detect two MAD_ERROR_BUFLEN occurrences at the same address without any other intervening repositioning of the input. The latter is arguably more safe, yet if mad_header_decode() had some way to indicate how much input it was expecting for the current frame, a cleaner solution is possible. This calculation of N bytes exists in the routine. Stuffing this into a new stream structure member would be one possibility..

Any suggestions?

Thanks,

-john

Rob Leslie wrote:

john cooper wrote:

...

For non-mmap'ed input implementations, I have encountered a

situation where due to a truncated last frame, an infinite loop occurs.

Where mad_header_decode() returning MAD_ERROR_BUFLEN is interpreted as "need more input", my input caching routine continues to return the unconsumed partial frame. mad_header_decode() says that's not enough and the process just goes on.

The complication I have here is my input routine is a random access input cache routine and doesn't know an EOF has truly occurred until input data is requested beyond the end of file. Thus seeking N times to a partial last frame (or any other frame) could happen.

I guess I'm a little confused: if MAD_ERROR_BUFLEN is a request for more data, why doesn't your code look for more data? Eventually you should request data beyond the end of file and trigger EOF?

Rob, In this case there is a truncated (corrupted) frame at the end of the input file. mad_header_decode() won't decode this as it correctly calls for more input. However in this case there is none.

My code knows nothing of the structure of an mp3 frame, thus can't tell there is a truncated final packet. I think the knowledge of frame structure is best left to MAD all else being equal.

A similar idea would be to monitor the amount of data you pass to mad_stream_buffer(); if it's the same amount as last time, and you received MAD_ERROR_BUFLEN without being able to decode a single frame last time, then you know it's not going to be enough this time. At this point you can try to read more data (and maybe find EOF) or you can pad with MAD_BUFFER_GUARD zero bytes and make a final attempt to decode the last frame.

I'm doing basically the same by keying off the logical seek pointer. Here again the input routine is not a simple streaming read of a file until EOF is encountered. It maintains a cache of file content in which the current seek pointer is contained. Arbitrary seeks within the input file are allowed and the cache window tracks the seek pointer. So if mad_header_decode() calls for input twice at the same seek pointer without any intervening user initiated stream seeks, I assume this to be an EOF on a corrupted frame.

I'm not discounting the notion that it could be helpful to know the expected frame size, but I'm not certain it's strictly necessary, and I don't see an obviously elegant way to implement it. (Adding a new member to the stream struct would be redundant most of the time since existing frame sizes can be calculated as stream->next_frame - stream->this_frame; it would also break shared library binary compatibility.)

I agree. My intention here was to find if I was overlooking something obvious. As there is a work-around to this problem, I thought you might want to add it to the list of things to address if you agree.

-john