On Mon, 16 Sep 2013, Josh Allmann wrote:
A given packet won't always come in contiguously; sometimes
they may be broken up on chunk boundaries by packets of another
channel.
This support primarily involves tracking information about the
data that's been read, so the reader can pick up where it left
off for a given channel.
As a side effect, we no longer over-report the bytes read if
(toread = MIN(size, chunk_size)) == size
---
libavformat/rtmppkt.c | 55 ++++++++++++++++++++++++++++++++++++-----------
libavformat/rtmppkt.h | 14 ++++++++++--
libavformat/rtmpproto.c | 9 +++++---
3 files changed, 60 insertions(+), 18 deletions(-)
This looks mostly good to me. Please squash patch #2 into this. As
commented in my answer to Luca, please make sure to free all intermediate
packets in the prev_pkt arrays in rtmp_close.
I do see one possibility for a pretty big simplification though...
diff --git a/libavformat/rtmppkt.c b/libavformat/rtmppkt.c
index 8f39122..7619d28 100644
--- a/libavformat/rtmppkt.c
+++ b/libavformat/rtmppkt.c
@@ -130,28 +130,32 @@ int ff_amf_read_null(GetByteContext *bc)
}
int ff_rtmp_packet_read(URLContext *h, RTMPPacket *p,
- int chunk_size, RTMPPacket *prev_pkt)
+ int chunk_size, RTMPPacket *prev_pkt,
+ int *retry_header)
{
uint8_t hdr;
- if (ffurl_read(h, &hdr, 1) != 1)
+ if (*retry_header >= 0) {
+ hdr = *retry_header & 0xFF;
+ *retry_header = -1;
+ } else if (ffurl_read(h, &hdr, 1) != 1)
return AVERROR(EIO);
- return ff_rtmp_packet_read_internal(h, p, chunk_size, prev_pkt, hdr);
+ return ff_rtmp_packet_read_internal(h, p, chunk_size, prev_pkt, hdr,
retry_header);
}
int ff_rtmp_packet_read_internal(URLContext *h, RTMPPacket *p, int chunk_size,
- RTMPPacket *prev_pkt, uint8_t hdr)
+ RTMPPacket *prev_pkt, uint8_t hdr,
+ int* retry_header)
{
uint8_t t, buf[16];
- int channel_id, timestamp, size, offset = 0;
+ int channel_id, timestamp, size;
uint32_t extra = 0;
enum RTMPPacketType type;
int written = 0;
int ret;
- written++;
channel_id = hdr & 0x3F;
if (channel_id < 2) { //special case for channel number >= 64
@@ -198,9 +202,26 @@ int ff_rtmp_packet_read_internal(URLContext *h, RTMPPacket
*p, int chunk_size,
if (hdr != RTMP_PS_TWELVEBYTES)
timestamp += prev_pkt[channel_id].timestamp;
+ if (!prev_pkt[channel_id].read) {
if ((ret = ff_rtmp_packet_create(p, channel_id, type, timestamp,
size)) < 0)
return ret;
+ // include 1-byte header from ff_rtmp_packet_read
+ p->read = written + 1;
+ p->offset = 0;
+ } else {
+ // previous packet in this channel hasn't completed reading
+ RTMPPacket *prev = &prev_pkt[channel_id];
+ p->data = prev->data;
+ p->size = prev->size;
+ p->channel_id = prev->channel_id;
+ p->type = prev->type;
+ p->ts_delta = prev->ts_delta;
+ p->extra = prev->extra;
+ p->offset = prev->offset;
+ p->read = prev->read + written;
+ prev->data = NULL;
+ }
p->extra = extra;
// save history
prev_pkt[channel_id].channel_id = channel_id;
@@ -209,26 +230,34 @@ int ff_rtmp_packet_read_internal(URLContext *h,
RTMPPacket *p, int chunk_size,
prev_pkt[channel_id].ts_delta = timestamp -
prev_pkt[channel_id].timestamp;
prev_pkt[channel_id].timestamp = timestamp;
prev_pkt[channel_id].extra = extra;
+ size = size - p->offset;
while (size > 0) {
int toread = FFMIN(size, chunk_size);
- if (ffurl_read_complete(h, p->data + offset, toread) != toread) {
+ if (ffurl_read_complete(h, p->data + p->offset, toread) != toread) {
ff_rtmp_packet_destroy(p);
return AVERROR(EIO);
}
size -= chunk_size;
- offset += chunk_size;
- written += chunk_size;
+ p->read += toread;
+ p->offset += toread;
if (size > 0) {
if ((ret = ffurl_read_complete(h, &t, 1)) < 0) { // marker
ff_rtmp_packet_destroy(p);
return ret;
}
- written++;
- if (t != (0xC0 + channel_id))
- return -1;
+ p->read++;
+ if (t != (0xC0 + channel_id)) {
+ RTMPPacket *prev = &prev_pkt[channel_id];
+ *retry_header = t;
+ prev->data = p->data;
+ prev->read = p->read;
+ prev->offset = p->offset;
+ return 0; // should lead to AVERROR(EAGAIN)
This does add a bit of assumptions; both that the rest of the RTMP
implementation really needs to handle 0 as AVERROR(EAGAIN) now, secondly
that the caller of this code needs to handle the AVERROR(EAGAIN) case as
well. (I do think that's handled properly within the ffurl_read function
though.) It doesn't seem to be handled right in the rtmp_write function
though.
One way to simplify things would be not to return 0 here, but goto the
start of the function again. Gotos aren't all that popular of course, so
instead of goto we would extend the loop to that whole scope.
This would allow getting rid of the retry_header variable altogether.
Or perhaps we'd need to split the function into a
rtmp_packet_read_one_chunk and have ff_rtmp_packet_read_internal as a thin
wrapper around that which does the looping.
I actually tried implementing this - I'll send my version of it, please
have a look and see if this looks sensible to you.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
