[FFmpeg-devel] [PATCH] Add Apple HTTP Live Streaming protocol handler

[Martin Storsjö]

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>