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

[Michel Bardiaux]

M?ns Rullg?rd a ?crit :
> Michel Bardiaux <mbardiaux at mediaxim.be> writes:
> 
>> Michael Niedermayer a ?crit :
>>> On Fri, Jan 04, 2008 at 05:50:01PM +0100, Michel Bardiaux wrote:
>>>> Michael Niedermayer a ?crit :
>>>>> On Thu, Jan 03, 2008 at 10:48:49PM +0000, Paul Kelly wrote:
>>>>>> Hello
>>>>>> I'm using libavformat to demux a continuous stream of MPEG2-PS
>>>>>> data and am running into the problem that memory usage steadily
>>>>>> increases over time. The problem is not observed when demuxing an
>>>>>> MPEG2-TS stream.
>>>>>>
>>>>>> After a bit of digging around I discovered the problem is caused
>>>>>> by the timestamp indexing in the PS demuxer - specifically, the
>>>>>> calls to av_add_index_entry() in mpegps_read_pes_header() in
>>>>>> libavformat/mpeg.c. All I'm doing is transcoding the stream to a
>>>>>> different output format and I don't need to be able to perform
>>>>>> seeking but there doesn't seem to be any way to disable the
>>>>>> index. (I guess the memory occupied by the index isn't a problem
>>>>>> if a fixed-size file is being demuxed, but in my case I am
>>>>>> reading data from a hardware MPEG encoder card and splitting the
>>>>>> output into separate files and the process is required to run
>>>>>> indefinitely - the index quickly grows to an unwieldy size.)
>>>>>>
>>>>>> As far as I can see the flag AVFMT_GENERIC_INDEX can be turned
>>>>>> off to stop indexing if generic indexing is used (perhaps that's
>>>>>> a non-standard usage though) - but is there no way to turn off
>>>>>> the indexing in the MPEG-PS demuxer?
>>>>>>
>>>>>> Might it be a good idea to add another flag to turn off the
>>>>>> demuxer-specific indexing, and make individual demuxers respect
>>>>>> this? A general catch-all way of disabling indexing (or
>>>>>> specifying that seeking isn't required) might be more elegant
>>>>>> though.
>>>>>>
>>>>>> I can get over the immediate problem by simply commenting out the
>>>>>> line calling av_add_index_entry() in libavformat/mpeg.c, but
>>>>>> would like to help get a better solution into libavformat if I
>>>>>> can.
>>>>> Disabling it with a flag is surely interresting. But i think
>>>>> there are better solutions.  One for example would be a
>>>>> max_index_size. And when thats reached index entries would be
>>>>> pseudo randomly droped. That would limit the used memory and
>>>>> still speed up seeking.
>>>>>
>>>> Another possibility (not exclusive):
>>>>
>>>> if(!url_is_streamed(s->pb)) av_add_index_entry(...)
>>>>
>>>> After all, if the input is streamed, the index is unlikely to be
>>>> of any use!
>>> patch welcome
>>>
>> Here it is if it is still welcome after all the messages exchanged
>> this week-end. It does not mean the max index size should not be
>> implemented too. And IMHO, yes, the same change should be made in most
>> containers, case by case. For AVI, certainly, since streaming an AVI
>> wont work anyway!
>>
>> And this issue led me to read more carefully the code of
>> mpegps_read_pes_header, and I must say it looks quite strange to me,
>> how can it work for streamed (http) MPEG-PS when it relies so much on
>> url_fseek, ftell, fskip, which should fail for a streamed URL, but the
>> return value is never checked?
>>
>> Index: libavformat/mpeg.c
>> ===================================================================
>> --- libavformat/mpeg.c	(revision 11441)
>> +++ libavformat/mpeg.c	(working copy)
>> @@ -390,7 +390,8 @@
>>      if(dts != AV_NOPTS_VALUE && ppos){
>>          int i;
>>          for(i=0; i<s->nb_streams; i++){
>> -            if(startcode == s->streams[i]->id) {
>> +            if(startcode == s->streams[i]->id &&
>> +               !url_is_streamed(s->pb) /* index useless on streams anyway */) {
>>                  av_add_index_entry(s->streams[i], *ppos, dts, 0, 0, AVINDEX_KEYFRAME /* FIXME keyframe? */);
>>              }
>>          }
> 
> The patch as such is OK, although it isn't entirely true that the
> index is useless when the streamed flag is set.  Some streaming
> sources do support seeking.
> 
Applied.

-- 
Michel Bardiaux
R&D Director
T +32 [0] 2 790 29 41
F +32 [0] 2 790 29 02
E mailto:mbardiaux at mediaxim.be

Mediaxim NV/SA
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
http://www.mediaxim.com/