[Ffmpeg-devel] [PATCH] split av_encode function and do related reorder
Michael Niedermayer
michaelni
Tue Mar 6 13:38:41 CET 2007
Hi
On Tue, Mar 06, 2007 at 11:24:04AM +0800, Limin Wang wrote:
> Hi,
>
> > > +static AVOutputStream **ost_table = NULL;
> > > +static AVInputStream **ist_table = NULL;
> > > +static AVInputFile *file_table = NULL;
> > > +static int nb_istreams = 0;
> > > +static int nb_ostreams = 0;
> >
> > these should stay local variables of av_encode and passed as arguments to
> > the other av_encode_*() functions
>
> The variables is initialized and allocate in av_encode_init(), so if we keep
> it as local variables of av_encode(), it'll cause av_encode_init() output
> these variables which will cause the code chaos.
well you could pass a pointer to them, and iam not saying you should just that
this would be one possible solution
> Current svn have no such
> issue for all variables is local. How about to move these variables to
> global as a separate patch before do the av_encode() split?
no, iam against making local variables global, i originally misstakely thought
they where global already before your patch ...
>
> >
> >
> > [...]
> > > int nb_input_files,
> > > AVStreamMap *stream_maps, int nb_stream_maps)
> > > {
> > > - int ret, i, j, k, n, nb_istreams = 0, nb_ostreams = 0;
> > > - AVFormatContext *is, *os;
> > > - AVCodecContext *codec, *icodec;
> > > - AVOutputStream *ost, **ost_table = NULL;
> > > - AVInputStream *ist, **ist_table = NULL;
> > > - AVInputFile *file_table;
> > > - AVFormatContext *stream_no_data;
> > > - int key;
> > > + int ret;
> > >
> > > + ret = av_encode_init();
> > > + if( ret < 0 ) {
> > > + fprintf(stderr, "av_encode_init failed\n");
> > > + exit(1);
> > > + }
> >
> > this is not a proper way to return an error from a function
> > also dont forget ALL the needed free and close a return -1 will
> > almost certainly be wrong, and this did work before your "cleanup"
>
> Sorry, I can't sure your meaning. If we define a context for av_encode, then
> if av_encode_init failed, it should return NULL context, the invoker don't
> need free anything, it's av_encode_init job.
yes but it doesnt free anything in case of failure with your patch
also if av_encode() fails it should return an error not exit()
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070306/3517b178/attachment.pgp>
More information about the ffmpeg-devel
mailing list