[FFmpeg-devel] [HACK] Remove MAX_STREAMS usages
Michael Niedermayer
michaelni
Fri Sep 3 10:31:40 CEST 2010
On Thu, Aug 12, 2010 at 10:13:20PM +0200, Aurelien Jacobs wrote:
> On Wed, Aug 11, 2010 at 02:41:49PM -0700, Baptiste Coudurier wrote:
> > Hi guys,
> >
> > On 8/11/10 8:28 AM, Aurelien Jacobs wrote:
> > >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
> > > [...]
> > >@@ -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.
>
> I thought about this too... And then I discarded the idea. I don't
> remember exactly why, but I guess I was just too lazy to change all
> usage of those vars...
> Anyway, I did it now. And it indeed looks a bit cleaner.
> I split this into 2 patches. First one moves those vars into a struct,
> and second one replace MAX_STREAMS usage by dynamic allocation.
>
> Aurel
> utils.c | 60 ++++++++++++++++++++++++++++--------------------------------
> 1 file changed, 28 insertions(+), 32 deletions(-)
> a5229b9380dc8439a65b43a92be6b442658230ed stream_info_array.diff
> commit 3604eda5ff964b98cde9585890a7e265eb5a1fa8
> Author: Aurelien Jacobs <aurel at gnuage.org>
> Date: Thu Aug 12 12:45:00 2010 +0200
>
> move stream info arrays into a struct to ease future dynamic allocation
ok
[...]
> utils.c | 27 ++++++++++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
> ef7b009d45960c34528c5657bd47da27ea317151 max_stream.diff
> commit 6056ed1b62ad76e16234ef5ef1a4a7c78d68f148
> Author: Aurelien Jacobs <aurel at gnuage.org>
> Date: Thu Aug 12 18:18:26 2010 +0200
>
> dynamically use nb_streams instead of static use of MAX_STREAMS
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index faad812..4bde019 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);
Before this chnage AVFMTCTX_NOHEADER dox should be clarified to say that
streams may only be added from the calling thread and not a background
thread otherwise this is a race and heap overflow.
i guess none of our demuxers adds streams from background threads, but
this too needs to be checked (mostly audio/video grabing demuxers seem
like they could by odd design do this in principle)
>
> @@ -2161,13 +2165,18 @@ int av_find_stream_info(AVFormatContext *ic)
> AVStream *st;
> AVPacket pkt1, *pkt;
> int64_t old_offset = url_ftell(ic->pb);
> + int nb_streams = ic->nb_streams;
> struct {
> int64_t last_dts;
> int64_t duration_gcd;
> int duration_count;
> double duration_error[MAX_STD_TIMEBASES];
> int64_t codec_info_duration;
> - } info[MAX_STREAMS] = {{0}};
> + } *info, *tmp_info;
> +
> + info = av_mallocz(ic->nb_streams * sizeof(*info));
> + if (!info)
> + return AVERROR(ENOMEM);
>
> for(i=0;i<ic->nb_streams;i++) {
> st = ic->streams[i];
> @@ -2198,7 +2207,7 @@ int av_find_stream_info(AVFormatContext *ic)
> }
> }
>
> - for(i=0;i<MAX_STREAMS;i++){
> + for(i=0;i<ic->nb_streams;i++){
> info[i].last_dts= AV_NOPTS_VALUE;
> }
>
> @@ -2264,8 +2273,19 @@ int av_find_stream_info(AVFormatContext *ic)
> break;
> }
>
> + if (ic->nb_streams > nb_streams) {
> + if (!(tmp_info = av_realloc(info, ic->nb_streams*sizeof(*info)))) {
missing checks for integer overflows and possibly not just here
that is unless the stream adding code limits to a small enough number AND
this limiting code is well documented to why it is needed and cant just be
removed
also the path over EAGAIN will at least crash if it adds more streams
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100903/3e1543e7/attachment.pgp>
More information about the ffmpeg-devel
mailing list