[FFmpeg-devel] [PATCH] Add Apple HTTP Live Streaming protocol handler
Martin Storsjö
martin
Thu Aug 5 22:20:15 CEST 2010
On Wed, 4 Aug 2010, Ronald S. Bultje wrote:
> On Tue, Jul 27, 2010 at 5:45 AM, Martin Storsj? <martin at martin.st> wrote:
> [..]
> > +struct segment {
> > + int duration;
> > + char *url;
> > +};
> > +
> > +struct variant {
> > + int bandwidth;
> > + char *url;
> > + ByteIOContext *pb;
> > + AVFormatContext *ctx;
> > + AVPacket pkt;
> > + int stream_offset;
> > +
> > + int start_seq_no;
> > + int n_segments;
> > + struct segment **segments;
> > + int needed;
> > +};
>
> Each of these (and below) could use at least a few doxy words to
> explain what they are. What is a segment? What is a variant?
Added some generic documentation and explanations here.
> Is it possible to make url in both segment and vriant fixed-length, so
> that we don't have to alloc so may individual elements? Not critical,
> but would be nice.
Changed to use fixed-length, statically allocated strings.
> > +typedef struct AppleHTTPContext {
> > + char playlisturl[2048];
>
> Unused (and kinda big).
Removed
> > +static int read_chomp_line(ByteIOContext *s, char *buf, int maxlen)
> > +{
> > + int len = ff_get_line(s, buf, maxlen);
> > + if (len > 0 && (buf[len-1] == '\r' || buf[len-1] == '\n'))
> > + buf[--len] = '\0';
> > + if (len > 0 && (buf[len-1] == '\r' || buf[len-1] == '\n'))
> > + buf[--len] = '\0';
> > + return len;
> > +}
>
> This could be simplified (if ...buf[len-1]=='\n' { if
> (buf[len-2]=='n') .. else .. }), but that's not too critical.
>
> You can presumably also chop off all space-like characters, assuming
> that's valid.
Simplified a bit by chomping using a loop, removing all space chars at the
end
> > +static int parse_playlist(AppleHTTPContext *c, const char *url,
> > + struct variant *var)
> > +{
> > + ByteIOContext *in;
> [..]
> > + if ((ret = url_fopen(&in, url, URL_RDONLY)) < 0)
> > + return ret;
>
> This should already be open in s->pb. Why do you want to reopen it?
Changed to potentially use an already open ByteIOContext, that is used for
the first call.
> > +static int applehttp_read_packet(AVFormatContext *s, AVPacket *pkt)
>
> This function could use some documentation / comments.
Added some narration to this function, too
> > + .extensions = "m3u8",
>
> ?? Really?
>
> Is a probe function possible based on the expected EXT-attributes in
> it that are relatively Apple-specific?
Removed the AVFMT_NOFILE flag, using the already open ByteIOContext for
parsing the first/topmost playlist the first time. Changed to use a proper
probe function, only reacting to the really apple specific tags.
New patch attached.
// Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-Apple-HTTP-Live-Streaming-demuxer.patch
Type: text/x-diff
Size: 22423 bytes
Desc:
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100805/e0910a4c/attachment.patch>
More information about the ffmpeg-devel
mailing list