[FFmpeg-devel] [PATCH] rtpdec: Better logic for immediately returning packets from the queue

[Martin Storsjö]

On Fri, 8 Oct 2010, Martin Storsj? wrote:

> On Fri, 8 Oct 2010, Luca Barbato wrote:
> 
> > On 10/08/2010 11:44 AM, Martin Storsj? wrote:
> > > On Fri, 8 Oct 2010, Luca Barbato wrote:
> > >
> > >> On 10/08/2010 11:16 AM, Martin Storsj? wrote:
> > >>> But what if packets 1-5 were one single large fragmented packet? Then the
> > >>> depacketization of packet 1 would return AVERROR(EAGAIN), and return no
> > >>> data until the whole frame has been depacketized. Currently, we'd return
> > >>> the control to the caller, and only proceed to check packet 2 the next
> > >>> time we're called. These patches instead make sure we check if we have the
> > >>> next packet in sequence, and try to parse that, if the current parsing
> > >>> returned<   0.
> > >>>
> > >>> I've tested this with the VP8 depacketizer that uses this logic, with an
> > >>> intentionally scrambled/reordered sender, and it seems to work as I intend
> > >>> now.
> > >>>
> > >>> Opinions?
> > >>
> > >> I'd check for EAGAIN explicitly, beside that it's fine for me.
> > >
> > > Not all of them return AVERROR(EAGAIN), some just return -1, too...
> > 
> > That might need attention.
> 
> True, and being able to return data sooner in this scenario is a good 
> incentive for returning proper error codes in the depacketizers.
> 
> > > And on
> > > the other side, even if packet 1 failed due to it being invalid, I still
> > > think we can try to depacketize the next one if we have it, and see if
> > > that one succeeds instead.
> > 
> > I think would be better notify this to the upper layer sooner and not 
> > hide it.
> 
> Fair enough. Like the attached, then?

Updated series attached, achieving the same thing, but in a slightly 
cleaner way. This avoids having to manually store prev_ret at every return 
statement in the code, which can easily be forgotten, by doing this in an 
outer function.

// Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-rtpdec-Split-out-storing-of-the-depacketization-retu.patch
Type: text/x-diff
Size: 4966 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101009/cc835bba/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-rtpdec-Don-t-call-the-depacketizer-to-return-more-da.patch
Type: text/x-diff
Size: 1390 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101009/cc835bba/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-rtpdec-Return-AVERROR-EAGAIN-if-out-of-data-for-mpeg.patch
Type: text/x-diff
Size: 1607 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101009/cc835bba/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-rtpdec-Parse-the-next-packet-in-the-sequence-if-it-i.patch
Type: text/x-diff
Size: 858 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101009/cc835bba/attachment-0003.patch>