[MPlayer-dev-eng] [PATCH] set is_streamed correctly in lavf URLContext

Michael Niedermayer michaelni at gmx.at
Wed Dec 19 14:05:07 CET 2007


On Wed, Dec 19, 2007 at 03:14:47AM +0100, Michael Niedermayer wrote:
> On Tue, Dec 18, 2007 at 10:54:24PM +0100, Aurelien Jacobs wrote:
> > Michael Niedermayer wrote:
> > 
> > > On Sat, Dec 15, 2007 at 05:35:19PM +0100, Reimar Döffinger wrote:
> > > > On Sat, Dec 15, 2007 at 05:33:05PM +0100, Reimar Döffinger wrote:
> > > > > On Sat, Dec 15, 2007 at 04:30:10PM +0100, Reimar Döffinger wrote:
> > > > > > I know it looks a bit hackish by modifying the file name.
> > > > > > A different way I just realized is of course having two
> > > > > > different protocols (only differing in the open function I
> > > > > > guess). Any opinions or better suggestions?
> > > > > 
> > > > > Actually this seems much better to me, it gets rid of the whole
> > > > > URLProtocol mess.
> > > > > The only problem is, it leaks memory since I could not find a
> > > > > function to just free the AVFormatContext, the currently used
> > > > > function can not be used since it wants to use url_fclose, which
> > > > > is only valid for URLProtocol stuff (I sent a mail about this to
> > > > > ffmpeg-dev)...
> > > > 
> > > > ...
> > > > 
> > > 
> > > > Index: libmpdemux/demux_lavf.c
> > > > ===================================================================
> > > > --- libmpdemux/demux_lavf.c	(revision 25408)
> > > > +++ libmpdemux/demux_lavf.c	(working copy)
> > > > @@ -60,11 +60,13 @@
> > > >  	{NULL, NULL, 0, 0, 0, 0, NULL}
> > > >  };
> > > >  
> > > > +#define BIO_BUFFER_SIZE 32768
> > > >  
> > > >  typedef struct lavf_priv_t{
> > > >      AVInputFormat *avif;
> > > >      AVFormatContext *avfc;
> > > > -    ByteIOContext *pb;
> > > > +    ByteIOContext pb;
> > > > +    uint8_t buffer[BIO_BUFFER_SIZE];
> > > 
> > > you assume that sizeof(ByteIOContext) will not change between lavf
> > > versions, or you just dont care that it will break even with minor
> > > version bumps ... :)
> > 
> > If sizeof(ByteIOContext) can change between lavf minor version, then
> > it shouldn't be part of public API.
> > Maybe the definition of ByteIOContext should be moved to a private
> > header and only the declaration should be kept in public header ?
> 
> Maybe ...

After thinking about this again and after reimars comments, i think yes we
should remove it, we can always add it back with a

#ifndef INTERNAL
uint8_t array[INT_MAX];
#endif
in it to make misuse harder

Mplayers lavc/lavf wrapers certainly serve as examples for other projects.
And if they are filled with ugly hacks which break lavcs ABI/API then this
will negatively affect more than just mplayer.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20071219/9d1bd219/attachment.pgp>


More information about the MPlayer-dev-eng mailing list