[FFmpeg-devel] MPEG-PS demuxer index memory usage

[Paul Kelly]

On Sat, 12 Jan 2008, Michael Niedermayer wrote:

> On Mon, Jan 07, 2008 at 08:10:17AM +0000, Paul Kelly wrote:
> [...]
>>  /**
>> + * Check the size of the index for the given stream against the maximum
>> + * specified in the AVFormatContext. If adding another entry would go
>> + * over the limit, halve the size of the index by removing every second
>> + * entry.
>> + *
>> + * @return < 0 if no more entries may be added to the index or the stream is invalid
>> + */
>
> Trailing whitespace, and it should be more generically described, not
> mentioning the precisse implementation.
> Like, "ensures that the index uses less mem than what is specified as
> maximimum in AVFormatContext.max_index_size. If the index is too large
> it will discard entries."

OK - I've simplified that doxygen comment and put the mention of halving 
the size back into the function definition as a comment - is that 
acceptable style? The FFmpeg source does seem pretty sparse on comments 
but I guess the philosophy is to simplify the code as much as possible so 
comments aren't needed..

> [...]
>> +int ff_reduce_index(AVFormatContext *s, int stream_index)
>> +{
>> +    AVStream *st;
>> +    unsigned int max_entries;
>> +
>
>> +    if(stream_index < 0 || stream_index >= s->nb_streams)
>> +        return -1;
>
> i think it can be assumed that the parameters are valid
>
>
>> +    st= s->streams[stream_index];
>> +
>> +    max_entries= s->max_index_size / sizeof(AVIndexEntry);
>
>> +    if(max_entries == 0)
>> +        return -1;
>
> I dont see what harm a single entry would do with max_entries == 0.
> So this and the return checking seems unneeded

OK I've removed them and made the function void. I thought it would save 
executing some code but I guess the performance benefit is insignificant 
compared to decoding video.

I think given Mans' comment that some streamed sources do support seeking, 
the other patch is not such a good idea. So now I'd prefer to see this one 
go in :)

Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ff_reduce_index2.patch
Type: text/x-diff
Size: 3626 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080112/d3fe5db8/attachment.patch>