[FFmpeg-devel] [RFC][PATCH] av_fifo_write_from_bytestream()

Björn Axelsson bjorn.axelsson
Thu Apr 3 16:57:07 CEST 2008


On Thu, 2008-04-03 at 14:31 +0200, Michael Niedermayer wrote:
> On Thu, Apr 03, 2008 at 11:41:23AM +0000, Bj?rn Axelsson wrote:
> > On Thu, 2008-04-03 at 13:06 +0200, Michael Niedermayer wrote:
> > > On Thu, Apr 03, 2008 at 12:14:36PM +0200, Bj?rn Axelsson wrote:
> > > > Hi list,
> > > > when changing the mms protocol over to use fifos I need a way to
> > > > efficiently read data from a ByteIOContext into an AVFifoBuffer. 
> > > > 
> > > > The attached patch adds such a function to fifo.(c|h) but also makes
> > > > those files depend on avformat.h
> > > 
> > > rejected
> > > see av_fifo_generic_read()
> > 
> > Right you are, and blind I am...
> > 
> > The attached patch implements av_fifo_generic_write() modeled after
> > av_fifo_generic_read().
> > 
> > I'll send a separate patch for reimplementing av_fifo_write() using
> > av_fifo_generic_write() when (if) this is accepted.
> 
> What about replacing av_fifo_write() by av_fifo_generic_write() and
> providing a wraper (maybe with attribute_deprecated)?
> If you now say thats what you are doing then look at your patch, you
> dont replace av_fifo_write() you add a new function.
> That is you add code duplication with the intent to remove the old code
> in a subsequent patch. This hides the changes between the old and new
> code.

ok, patches merged.

I don't know if an attribute_deprecated should be used on the old
av_fifo_write() 
Personally I think not, because it is a nice and simple convenience
function.

> > I must note that I am a bit unhappy with the error handling of this
> > implementation, but it is consistent with the rest of the AVFifoBuffer
> > API.
> 
> Consistency is no excuse to leave a problem unsolved.

If you insist =). Patch extended with what I consider minimal error
handling. I also extended the documentation of the contract for the
callback function.

Some discussion points:
Should we restore the fifo, i.e. reset the wptr to its original value,
in case of an error from the callback function?
That, together with an initial check that the amount of data actually
fits in the fifo, would guarantee that the fifo state is unchanged after
a failed av_fifo_generic_write() call.
However, the added size check would break any callers of av_fifo_write()
that rely on it not being there.

-- 
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: av_fifo_generic_write.diff
Type: text/x-patch
Size: 2249 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080403/165ae252/attachment.bin>



More information about the ffmpeg-devel mailing list