[FFmpeg-devel] [PATCH] Remove static ByteIOContexts

Björn Axelsson bjorn.axelsson
Thu Nov 15 11:46:51 CET 2007 

On Wed, 2007-11-07 at 03:18 +0100, Michael Niedermayer wrote:
> On Tue, Nov 06, 2007 at 03:42:22PM +0100, Bj?rn Axelsson wrote:
> > To facilitate future extensions of ByteIOContext without breaking the
> > ABI Michael suggested[1] changing
> > AVFormatContext{
> >     ByteIOContext pb;
> > to
> > AVFormatContext{
> >     ByteIOContext *pb;
> > 
> > This is my attempt at doing that. 
> > 
> > Basically url_fopen(), url_fdopen(), url_open_buf and url_open_dyn_*()
> > are all changed to take a ByteIOContext ** pointer which is used to
> > return a newly allocated ByteIOContext in. This is consistent with how
> > url_open() works with URLContexts.
> 
> > 
> > I believe this patch will break compability with most applications,
> 
> you need to increase the major version part of LIBAVFORMAT_VERSION*

version bump patch now included (versionbump.diff)

> > since AVFormatContext.pb must be checked for NULL before accessed (if a
> > format with AVFMT_NOFILE is used). Both ffmpeg and ffplay needed
> > additional checks for this. Of course, any application using the changed
> > url_* functions will also need some minor changes.
> 
> iam not overly happy about these NULL checks
> wouldnt having url_fseek() return EINVAL or something like that for NULL
> contextes instead of crashing avoid most of these NULL checks ?

I don't feel that either solution is perfect, but in this patch series
most null checks have been moved to aviobuf instead. url_fseek() and
url_fsize() now return EINVAL, while  url_feof() and url_ferror() both
return 0 when called with a NULL parameter. 
That is the minimal number of patched functions to make ffmpeg and
ffplay work.

> [...]

> > Index: libavformat/aviobuf.c
> > ===================================================================
> > --- libavformat/aviobuf.c.orig	2007-11-06 13:20:58.466007062 +0100
> > +++ libavformat/aviobuf.c	2007-11-06 13:28:05.390808599 +0100
> > @@ -508,12 +508,11 @@
> >      //return 0;
> >  }
> >  
> > -int url_fdopen(ByteIOContext *s, URLContext *h)
> > +int url_fdopen(ByteIOContext **s, URLContext *h)
> >  {
> 
> >      uint8_t *buffer;
> >      int buffer_size, max_packet_size;
> >  
> > -
> >      max_packet_size = url_get_max_packet_size(h);
> 
> cosmetic

I must have thought that the double empty line was my fault and removed
it. Patch fixed.

> [...]
> 
> > Index: ffmpeg_avformatcontext/libavformat/utils.c
> > ===================================================================
> > --- ffmpeg_avformatcontext.orig/libavformat/utils.c	2007-11-06 14:32:23.144460497 +0100
> > +++ ffmpeg_avformatcontext/libavformat/utils.c	2007-11-06 15:23:54.708850579 +0100
> > @@ -1079,6 +1079,9 @@
> >      int index;
> >      AVStream *st;
> >  
> > +    if(s->pb == NULL)
> > +        return -1;
> > +
> >      if (stream_index < 0)
> >          return -1;
> >  
> > @@ -1141,6 +1144,9 @@
> >      int64_t start_pos, filesize;
> >      int no_change;
> >  
> > +    if(! s->pb)
> > +        return -1;
> 
> inconsistant, you are mixing ! and ==NULL, i prefer !

All NULL tests changed to avoid == NULL or != NULL.

Patches synced with current SVN, and the full series still passes
regression tests.

Slightly revised patch series and suggested commit messages:
 
 1. bioc_api_1.diff
    Dynamic allocation of the ByteIOContext in AVFormatContexts
 2. bioc_formats_1.diff
    Update libavformats for dynamic ByteIOContexts
 3. bioc_utils.diff
    Update utils.c for dynamic ByteIOContexts
 4. bioc_aviobuf_nullcheck.diff
    Handle NULL parameters in url_fseek(), url_fsize(), url_ferror() and url_feof()
 5. bioc_ffplay-2.diff
    Update ffplay for dynamic ByteIOContexts
 6. bioc_ffplay_reindent.diff
    Reindent from previous patch
 7. bioc_ffmpeg-2.diff
    Update ffmpeg for dynamic ByteIOContexts
 8. bioc_ffserver.diff
    Update ffserver for dynamic ByteIOContexts
 9. bioc_dynbuf_api.diff
    Let url_open_buf() and url_open_dyn_*() allocate ByteIOContexts
10. bioc_dynbuf_formats.diff
    Update libavformats for url_open_buf() and url_open_dyn_*() changes
11. bioc_dynbuf_ffserver.diff
    Update ffserver for url_open_buf() and url_open_dyn_*() changes
12. versionbump.diff
    Increase libavformat major version

-- 
Bj?rn Axelsson                    Phone: +46-(0)90-18 98 97
Intinor AB                          Fax: +46-(0)920-757 10
www.intinor.se
Interactive Television & Digital Media Distribution
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_api_1.diff
Type: text/x-patch
Size: 2899 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_formats_1.diff
Type: text/x-patch
Size: 157300 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_utils.diff
Type: text/x-patch
Size: 6114 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_aviobuf_nullcheck.diff
Type: text/x-patch
Size: 1047 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_ffplay-2.diff
Type: text/x-patch
Size: 2016 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_ffplay_reindent.diff
Type: text/x-patch
Size: 550 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_ffmpeg-2.diff
Type: text/x-patch
Size: 846 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_ffserver.diff
Type: text/x-patch
Size: 753 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_dynbuf_api.diff
Type: text/x-patch
Size: 3770 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_dynbuf_formats.diff
Type: text/x-patch
Size: 7592 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bioc_ffserver.diff
Type: text/x-patch
Size: 753 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0010.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: versionbump.diff
Type: text/x-patch
Size: 624 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071115/5836a110/attachment-0011.bin>

More information about the ffmpeg-devel mailing list