[FFmpeg-devel] [PATCH] avformat/mov: read stream bitrates from isml manifest

[Michael Niedermayer]

On Wed, Jul 03, 2013 at 05:58:03PM +0200, Alexandre Sicard wrote:
> Le jeudi 27 juin 2013 15:25:02 Michael Niedermayer a écrit :
> > On Thu, Jun 27, 2013 at 12:16:07PM +0200, Alexandre Sicard wrote:
> > > Le jeudi 27 juin 2013 10:36:11 Michael Niedermayer a écrit :
> > > > > +#include <regex.h>
> > > > 
> > > > This is unavaliable on at least windows AFAIK
> > > 
> > > Oops, this is a problem. Parsing XML would require regex or an XML parsing
> > > library. Actually, I first wrote this patch using libexpat, but I thought
> > > it was not worth adding a dependency when I could just use regex.
> > > 
> > > Now, I think it could be useful: I have a colleague working on adding
> > > Smooth Streaming playback support to FFmpeg, and for now he uses libexpat
> > > to parse the manifest. Other adaptive streaming formats such as DASH and
> > > HDS have XML manifests too, so it could definitively be of use.
> > > 
> > > What do you think about this? Should I send a patch adding libexpat to
> > > FFmpeg and the libexpat version of my patch?
> > 
> > does this really need more than 5 lines of C code with sscanf() ?
> > 
> > [...]
> 
> Indeed, the bitrates are quite easy to read even without a parser. Reading 
> more fields could be tricky though. We can use string functions for now, see 
> updated patch.
> 
> > > @@ -3221,6 +3293,12 @@ static int mov_read_header(AVFormatContext *s)
> > > 
> > >          }
> > >      
> > >      }
> > > 
> > > +    for (i = 0; i < mov->bitrates_count && i < s->nb_streams; i++) {
> > > +        if (mov->bitrates[i] > 0) {
> > > +            s->streams[i]->codec->bit_rate = mov->bitrates[i];
> > > +        }
> > > +    }
> > 
> > Is this indirection neccessary (compared to directly writing into
> > s->streams[i]->codec->bit_rate)
> > ?
> 
> The uuid box containing the manifest is the first box in the file. That means s-
> >streams is null when it is read, so I can't directly write to it.
> 
> -- 
> Alexandre SICARD - Intern, Research & Engineering
> SmartJog | www.smartjog.com | a TDF Group company

>  isom.h |    2 +
>  mov.c  |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+)
> c36dc6c9eafed06c2d60a40a87b79d40a5b94253  0001-avformat-mov-read-stream-bitrates-from-isml-manifest.patch
> From f07712d5442a246b12fa51766a9e19fd476b5d4a Mon Sep 17 00:00:00 2001
> From: Alexandre Sicard <alexandre.sicard at smartjog.com>
> Date: Tue, 2 Jul 2013 11:59:52 +0200
> Subject: [PATCH] avformat/mov: read stream bitrates from isml manifest
> 
> This allows to read a live isml movie and segment it using the
> smoothstreaming muxer, which requires the bitrates to be known for each stream.
> 
> Signed-off-by: Alexandre Sicard <alexandre.sicard at smartjog.com>
> ---
>  libavformat/isom.h |  2 ++
>  libavformat/mov.c  | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+)
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index 220b2df..b0fa453 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -162,6 +162,8 @@ typedef struct MOVContext {
>      int use_absolute_path;
>      int ignore_editlist;
>      int64_t next_root_atom; ///< offset of the next root atom
> +    int *bitrates;          ///< bitrates read before streams creation
> +    int bitrates_count;
>  } MOVContext;
>  
>  int ff_mp4_read_descr_len(AVIOContext *pb);
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 37030a5..0bb11a0 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -2731,6 +2731,68 @@ static int mov_read_tmcd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      return 0;
>  }
>  
> +static int mov_read_uuid(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> +{
> +    int ret;
> +    uint8_t uuid[16];
> +    static const uint8_t uuid_isml_manifest[] = {
> +        0xa5, 0xd4, 0x0b, 0x30, 0xe8, 0x14, 0x11, 0xdd,
> +        0xba, 0x2f, 0x08, 0x00, 0x20, 0x0c, 0x9a, 0x66
> +    };
> +
> +    if (atom.size < sizeof(uuid) || atom.size == INT64_MAX)
> +        return AVERROR_INVALIDDATA;
> +
> +    ret = avio_read(pb, uuid, sizeof(uuid));
> +    if (ret < 0) {
> +        return ret;
> +    } else if (ret != sizeof(uuid)) {
> +        return AVERROR_INVALIDDATA;
> +    }
> +    if (!memcmp(uuid, uuid_isml_manifest, sizeof(uuid))) {
> +        uint8_t *buffer, *ptr;
> +        char *endptr;
> +        size_t len = atom.size - sizeof(uuid);
> +
> +        avio_skip(pb, 4); // zeroes
> +        len -= 4;

missing check that len >= 4


> +
> +        buffer = av_mallocz(len + 1);
> +        if (!buffer) {
> +            return AVERROR(ENOMEM);
> +        }
> +        ret = avio_read(pb, buffer, len);
> +        if (ret < 0) {
> +            av_free(buffer);
> +            return ret;
> +        } else if (ret != len) {
> +            av_free(buffer);
> +            return AVERROR_INVALIDDATA;
> +        }
> +
> +        ptr = buffer;
> +        while ((ptr = strcasestr(ptr, "systemBitrate=\"")) != NULL) {
> +            ptr += sizeof("systemBitrate=\"") - 1;
> +            c->bitrates_count++;

> +            c->bitrates = av_realloc(c->bitrates, sizeof(*c->bitrates) * c->bitrates_count);

see av_realloc_f()


> +            if (!c->bitrates) {

here c->bitrates is NULL but c->bitrates_count might be >0
which is inconsistent

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130703/d6b4c378/attachment.asc>