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

[Måns Rullgård]

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.

-- 
M?ns Rullg?rd
mans at mansr.com