[FFmpeg-soc] [PATCH] xiph packetizer

[Martin Storsjö]

On Thu, 5 Aug 2010, Josh Allmann wrote:

> >> +
> >> +    if (!frag && !xdt) { // do we have a whole frame of raw data?
> >> +        int remaining = max_pkt_size - ((int)(s->buf_ptr - s->buf) + size);
> >
> > This calculation is slightly off - max_pkt_size already has the 6 bytes
> > header subtracted, but those 6 bytes still are included in s->buf_ptr - s->buf.
> > Also, it feels a bit convoluted.
> >
> > If you happen to have a packet of e.g. 1453, this forces the lines below
> > to send the currently buffered data, even if there isn't any frame buffered
> > (s->buf_ptr - s->buf is 3 bytes, and only contains the ident header).
> >
> 
> Hmm, OK. I tightened it up. The total header size is actually 4 + 2n,
> where n is the number of frames in the packet -- we need to prefix the
> length before each frame.
> 
> In this case though, those additional headers are included in
> (s->buf_ptr - s->buf), but we do need to have room for an additional 2
> bytes if the current frame is not the first of the packet.
> 
> I am not 100% certain this fix is correct, so a second pair of eyes
> would be good.

I'll have another look in a while.

> +        int data_size = (s->buf_ptr - s->buf) + size;
> +        int header_size = s->num_frames ? 8 : 6; // 6 bytes standard,
> 2 more bytes for length of extra frame
> +        int remaining = max_pkt_size - data_size - header_size;
> 
> >> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> >> index 689ad29..1be3cd0 100644
> >> --- a/libavformat/rtsp.c
> >> +++ b/libavformat/rtsp.c
> >> @@ -52,7 +52,7 @@ int rtsp_default_protocols = (1 << RTSP_LOWER_TRANSPORT_UDP);
> >>  #define SELECT_TIMEOUT_MS 100
> >>  #define READ_PACKET_TIMEOUT_S 10
> >>  #define MAX_TIMEOUTS READ_PACKET_TIMEOUT_S * 1000 / SELECT_TIMEOUT_MS
> >> -#define SDP_MAX_SIZE 8192
> >> +#define SDP_MAX_SIZE 16384
> >>
> >
> > Is there any limit on the size of the extradata for theora/vorbis?
> > Can we be reasonably sure this is enough for at least one theora stream
> > plus one vorbis stream?
> >
> 
> I am assuming you mean one theora stream, OR one vorbis stream, not
> AND. sdp_parse in rtsp.c (line 397) claims 16KB max for the FMTP line,
> but I'm not sure what the provenance of that number is. I took a quick
> glance through the Theora and Vorbis bitstream specs, and couldn't
> find any hard figures for this. From empirical testing, the Theora
> extradata is usually a bit smaller than the Vorbis. Even then, I have
> yet to see (non-base64-encoded) sizes of more than 5KB. So we should
> be comfortably under this limit.

No, I ment AND - if you stream both and audio video, the description of 
both go into the same single SDP block, so it has to be large enough for 
both of them. The comment at line 397 in rtsp.c refers to one single SDP 
line, so if there are cases where a vorbis line actually could be almost 
16 KB, we'd need yet a bit more space if we'd want to stream theora at the 
same time.


I'll have a look at the rest later.

// Martin