john cooper wrote:
For non-mmap'ed input implementations, I have encountered asituation 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?
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?
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 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.)