[FFmpeg-devel] [PATCH] RTSP-MS 14/15: ASF packet parsing

[Michael Niedermayer]

On Tue, Apr 21, 2009 at 08:47:40AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> (Michael before you start reviewing this patch, this one is not for
> your review (yet), it just fixes one of Luca's concerns.)
> 
> On Sat, Apr 18, 2009 at 6:08 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Sat, Apr 18, 2009 at 03:28:25PM -0400, Ronald S. Bultje wrote:
> > you do not explain why you reach an invalid internal state (ake EOF
> > in the middle of the stream) you explain only that because the state
> > is not yet invalid the demuxer tries to contiunue and then it becomes
> > invalid (=EOF) and that you now want to break out but thats too late,
> >
> > we first need to discuss WHY it becomes EOF and then either if
> > A. this can be avoided (does the demuxer read more than it needs, or
> > ? does the RTP code not provide enough data for the ASF demuxer to
> > ? return a single frame)
> > ? its very unclear from your description if you pass enough data
> > ? or not but my guess is you do not.
> > B. Change the ASF demuxer to support failing with EAGAIN on unavailable
> > ? next packet, and before you attempt it no, you cannot treat url_feof()
> > ? there by return EAGAIN that could end in an infinite loop if its real
> > ? EOF
> >
> > either way your eof check is broken and wrong, for example at the point
> > where you break out you could already have read 2 bytes from the next
> > packet successfully and i am not sure if the demuxer would recover from
> > that.
> 
> OK, so let's head on to the complex stuff now. Attached is the same
> patch as last time, now using url_*_dyn_buf() instead of that
> ff_rtp_merge_data_packet() function (thanks Luca A.!), no need to
> review because it still has the same issue as per above.
> 
> OK, let's answer this question so that we can decide what to do next
> to solve this issue. So the invalid state (EOF) is reached because I
> feed the demuxer individual packets, which naturally reach
> end-of-buffer ("EOF") at the end of each packet rather than only at
> the end of a file.
> 
> So my ByteIOContext for a file contains:
> <Header><data><Header><data><Header><data>[..]<EOF>
> 
> my RTP payloads and ByteIOContext for RTP contain:
> <Header><data><EOF>
> and then the next ByteIOContext again contains:
> <Header><data><EOF>
> and so on ([..]) until the RTSP stream stops. What you mention in your
> last paragraph (reading 2 bytes from the next frame) will thus not
> occur (I think) because that packet has simply not been loaded yet.
> The ASF packets appear perfectly framed between the RTP payloads.
> 
> OK, so on to the potential solutions. Can this invalid EOF state be
> avoided? Probably...

> - I could, for example, add a check in rtp_asf.c to automatically load
> the next packet if the position in the ByteIOContext is exactly the
> last byte in the current packet. However, then you get in the
> situation of "what to do if we're one byte away from the
> end-of-packet"? I haven't tried implementing it because it seems
> breakable to me. We'd still have the same problem, it'd only work for
> "perfect" streams, no error concealment, any single stream problem
> would lead to the issue that we're seeing here now. So I'd rather not
> go that way. Do you agree?

iam not 100% sure what you suggest but it does not sound like a good
idea


> 
> - it's essentially true that we're not feeding the demuxer enough data
> to read a single frame, because we basically don't know how much data
> to feed the demuxer to read a single frame. A video frame could be
> spread over multiple ASF packets (RTP payloads), but an audio packet
> could contain multiple frames... This doesn't seem very promising to
> me.

yes


> 
> - what my patch (as you said, probably wrongly) does is to make sure
> the demuxer doesn't attempt to read more when we've reached this
> "end-of-packet" / "end-of-buffer" / "end-of-payload" state. Probably
> not right either, but it was simpler to implement without being
> terribly breakable (just my personal impression, based no nothing)...
> 
> So although it's doable, I don't think it's terribly promising if
> we're looking for a stable way to solve this... Is your solution (B)
> more appropriate, or should I change my packet loading to get
> something in the direction of (A)? If (B), how would you suggest I do
> that?

I belive it would be very difficult for the rtp code to know without
interaction from the asf demuxer how much data it might want.

There seem 2 variants and i belive both should be implemented because
they are both usefull in different circumstances.

The first is that the rtp code should provide a fully functional
ByteIOContext, not one that has a EOF before the end of the file.
That means it must provide a read_packet() callback.
Such callback would of course block until the data has been received.
This first variant is usefull for a multithreaded application that runs
the demuxer in a seperate thread and is thus unaffected by the blocking.

The second is that when no more data is available this
state should be signaled to the demuxer (iam not sure if we have any
means to do this yet) also iam not sure which way is the nicest to
do this
a url_fwouldblock() could be used, that is the asf demuxer could do
if(url_fwouldblock(pb))
    return AVERROR(EAGAIN);
before reading the first byte of the next packet
internally url_fwouldblock() would have to somehow figure out if the
ByteIOContexts read_packet() would block or not, this may need adding
a would_block() callback
but iam less certain on how to design/implement all that ideally, thus
comments welcome!
This second variant is usefull for single threaded apps for which blocking
would not be acceptable.

Either way comments and alternative suggestions are definitly welcome.

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

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- 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/ffmpeg-devel/attachments/20090421/f86af1bd/attachment.pgp>