[FFmpeg-devel] [HACK] Remove MAX_STREAMS usages

Baptiste Coudurier baptiste.coudurier
Wed Aug 11 23:41:49 CEST 2010


Hi guys,

On 8/11/10 8:28 AM, Aurelien Jacobs wrote:
> On Tue, Aug 10, 2010 at 07:19:06PM +0200, Reimar D?ffinger wrote:
>> On Tue, Aug 10, 2010 at 07:31:49PM +0200, Michael Niedermayer wrote:
>>> On Tue, Aug 10, 2010 at 01:14:56AM +0200, Aurelien Jacobs wrote:
>>>> On Sun, Aug 08, 2010 at 10:40:40PM +0200, Reimar D?ffinger wrote:
>>>>> Hello,
>>>>> this is just a quick hack that removes all usages of
>>>>> MAX_STREAMS.
>>>>>
>>>>> [...]
>>>>
>>>> And here are the last bits I extracted/reworked from this patch.
>>>>
>>>> First patch in this email uses nb_streams instead of MAX_STREAMS to
>>>> dynamically alloc arrays in libavformat/utils.c. This is compatible
>>>> with both the old API and the new API.
>>>>
>>>> Second and last patch actually add the new API, finally getting ride of
>>>> MAX_STREAMS. It is obviously disabled until next major bump.
>>>>
>>>> Aurel
>>>>   utils.c |   37 +++++++++++++++++++++++++++----------
>>>>   1 file changed, 27 insertions(+), 10 deletions(-)
>>>> 553bd8bafb70b124e3ed2747f577f4ae057e8094  max_stream.diff
>>>> commit f04d73bbdcb67af2e6d1145c652a6b2978c0c83f
>>>> Author: Aurelien Jacobs<aurel at gnuage.org>
>>>> Date:   Tue Aug 10 00:21:06 2010 +0200
>>>>
>>>>      dynamically use nb_streams instead of static use of MAX_STREAMS
>>>>
>>>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>>>> index 1aa965c..edd3d19 100644
>>>> --- a/libavformat/utils.c
>>>> +++ b/libavformat/utils.c
>>>> @@ -1871,10 +1871,13 @@ static void av_estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset
>>>>       AVPacket pkt1, *pkt =&pkt1;
>>>>       AVStream *st;
>>>>       int read_size, i, ret;
>>>> -    int64_t end_time, start_time[MAX_STREAMS];
>>>> +    int64_t end_time, *start_time;
>>>>       int64_t filesize, offset, duration;
>>>>       int retry=0;
>>>>
>>>> +    if (!(start_time = av_malloc(ic->nb_streams * sizeof(*start_time))))
>>>> +        return;
>>>> +
>>>>       ic->cur_st = NULL;
>>>>
>>>>       /* flush packet queue */
>>>> @@ -1935,6 +1938,7 @@ static void av_estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset
>>>>       }while(   end_time==AV_NOPTS_VALUE
>>>>              &&  filesize>  (DURATION_MAX_READ_SIZE<<retry)
>>>>              &&  ++retry<= DURATION_MAX_RETRY);
>>>> +    av_free(start_time);
>>>>
>>>>       fill_all_stream_timings(ic);
>>>>
>>>> @@ -2160,15 +2164,23 @@ int av_find_stream_info(AVFormatContext *ic)
>>>>       int i, count, ret, read_size, j;
>>>>       AVStream *st;
>>>>       AVPacket pkt1, *pkt;
>>>> -    int64_t last_dts[MAX_STREAMS];
>>>> -    int64_t duration_gcd[MAX_STREAMS]={0};
>>>> -    int duration_count[MAX_STREAMS]={0};
>>>> +    int64_t *last_dts;
>>>> +    int64_t *duration_gcd;
>>>> +    int *duration_count;
>>>>       double (*duration_error)[MAX_STD_TIMEBASES];
>>>>       int64_t old_offset = url_ftell(ic->pb);
>>>> -    int64_t codec_info_duration[MAX_STREAMS]={0};
>>>> +    int64_t *codec_info_duration;
>>>>
>>>> -    duration_error = av_mallocz(MAX_STREAMS * sizeof(*duration_error));
>>>> -    if (!duration_error) return AVERROR(ENOMEM);
>>>> +    last_dts            = av_mallocz(ic->nb_streams * sizeof(*last_dts));
>>>> +    duration_gcd        = av_mallocz(ic->nb_streams * sizeof(*duration_gcd));
>>>> +    duration_count      = av_mallocz(ic->nb_streams * sizeof(*duration_count));
>>>> +    duration_error      = av_mallocz(ic->nb_streams * sizeof(*duration_error));
>>>> +    codec_info_duration = av_mallocz(ic->nb_streams * sizeof(*codec_info_duration));
>>>> +    if (!last_dts || !duration_gcd || !duration_count || !duration_error ||
>>>> +        !codec_info_duration) {
>>>> +        ret = AVERROR(ENOMEM);
>>>> +        goto err_out;
>>>> +    }
>>>
>>> possibly exploitable
>>> the number of streams can increase any time if more are found
>>
>> "any time" makes it sound more serious than it is, within that
>> function I think it is only in av_read_frame_internal, so
>> after that call resizing the arrays may be necessary.
>> Still going to be a bit annoying to implement but not that bad.
>
> Here is a fixed version which will grow the size of the buffers when
> new streams are added.
>
> Aurel
>
>
> max_stream.diff
>
>
> commit eb4a2bfc996713e70f60b556ed776cae3ab1cc5b
> Author: Aurelien Jacobs<aurel at gnuage.org>
> Date:   Tue Aug 10 00:21:06 2010 +0200
>
>      dynamically use nb_streams instead of static use of MAX_STREAMS
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 1aa965c..10e817d 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -1871,10 +1871,13 @@ static void av_estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset
>       AVPacket pkt1, *pkt =&pkt1;
>       AVStream *st;
>       int read_size, i, ret;
> -    int64_t end_time, start_time[MAX_STREAMS];
> +    int64_t end_time, *start_time;
>       int64_t filesize, offset, duration;
>       int retry=0;
>
> +    if (!(start_time = av_malloc(ic->nb_streams * sizeof(*start_time))))
> +        return;
> +
>       ic->cur_st = NULL;
>
>       /* flush packet queue */
> @@ -1935,6 +1938,7 @@ static void av_estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset
>       }while(   end_time==AV_NOPTS_VALUE
>              &&  filesize>  (DURATION_MAX_READ_SIZE<<retry)
>              &&  ++retry<= DURATION_MAX_RETRY);
> +    av_free(start_time);
>
>       fill_all_stream_timings(ic);
>
> @@ -2160,15 +2164,34 @@ int av_find_stream_info(AVFormatContext *ic)
>       int i, count, ret, read_size, j;
>       AVStream *st;
>       AVPacket pkt1, *pkt;
> -    int64_t last_dts[MAX_STREAMS];
> -    int64_t duration_gcd[MAX_STREAMS]={0};
> -    int duration_count[MAX_STREAMS]={0};
> -    double (*duration_error)[MAX_STD_TIMEBASES];
> +    int64_t *last_dts = NULL;
> +    int64_t *duration_gcd = NULL;
> +    int *duration_count = NULL;
> +    double (*duration_error)[MAX_STD_TIMEBASES] = NULL;
>       int64_t old_offset = url_ftell(ic->pb);
> -    int64_t codec_info_duration[MAX_STREAMS]={0};
> +    int64_t *codec_info_duration = NULL;
> +    int nb_streams = 0;
> +    void *tmp;
>
> -    duration_error = av_mallocz(MAX_STREAMS * sizeof(*duration_error));
> -    if (!duration_error) return AVERROR(ENOMEM);
> +#define GROW_BUFFER(buf, old_size, new_size)                           \
> +    if (!(tmp = av_realloc(buf, new_size * sizeof(*buf)))) {           \
> +        ret = AVERROR(ENOMEM);                                         \
> +        goto err_out;                                                  \
> +    }                                                                  \
> +    buf = tmp;                                                         \
> +    memset(buf+old_size, 0, (new_size-old_size) * sizeof(*buf));       \
> +
> +#define GROW_BUFFERS()                                                 \
> +    if (ic->nb_streams>  nb_streams) {                                 \
> +        GROW_BUFFER(last_dts,            nb_streams, ic->nb_streams);  \
> +        GROW_BUFFER(duration_gcd,        nb_streams, ic->nb_streams);  \
> +        GROW_BUFFER(duration_count,      nb_streams, ic->nb_streams);  \
> +        GROW_BUFFER(duration_error,      nb_streams, ic->nb_streams);  \
> +        GROW_BUFFER(codec_info_duration, nb_streams, ic->nb_streams);  \
> +    }                                                                  \

I suggest to define a struct per stream to simplify the code.

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org



More information about the ffmpeg-devel mailing list