[FFmpeg-devel] [PATCH] MXF index table based seeking

Baptiste Coudurier baptiste.coudurier at gmail.com
Tue May 17 05:31:36 CEST 2011


Hi Tomas,

On 5/16/11 1:27 AM, Tomas Härdin wrote:
> Georg Lippitsch skrev 2011-05-15 15:35:
>> Here is my first try to add image table based seeking to the MXF demuxer.
>>
>> What the patch basically does is:
>> 1. Parse through the whole file in mxf_read_header (instead of breaking
>> at the first essence container). If an index table is found, it is
>> parsed and stored in an MXFIndexTableSegment.
> 
> This seems OK, but it'll be very slow for large frame wrapped files.
> 
> What you could do to speed it up is to make it partition aware.
> Unfortunately the partitions only contain FooterPartition and
> PreviousPartition offsets, which means one would have to parse the file
> partly in reverse (you can't rely on there being a Random Index Pack).
> 
> I've been toying around with doing something like that. The logic is
> rather tricky though. I parse forward until I run into essence, then I
> seek to the last partition. After that I parse forward and seek to the
> previous partition if I run into essence or another partition. This
> repeats until I get back to where I was before I did the seek to the
> footer.
> 
> Rebased patches attached. The first one simply adds some code that
> parses MXF partitions as it runs across them. The second one does the
> parsing I explained above. The commit message is due to the exact same
> problem your patch has - my code didn't handle 11 GiB frame wrapped
> files very well :o
> 
>> 2. In mxf_parse_structural_metadata, the MXFIndexTableSegments are
>> parsed and an AVIndexEntry array is created from them
>> Seeking is then done based on the AVIndexEntry array.
> 
> Sounds OK.
> 
>> What I didn't really understand is how to link between MXF tracks and
>> the slices in the index table. Sometimes, there is a track for every
>> slice in the index table, sometimes there is not. So this part of the
>> code is probably wrong, but help on this is very appreciated!
> 
> The MXF book attempts to explain slices, but I still haven't managed to
> figure out how they work. What I can say is that they're for handling
> mixing CBR and VBR media, which is a fairly typical use case (VBR video,
> CBR audio).
> 
> I can't comment too much on the index parsing code since I don't quite
> understand how MXF's indexes work.
> 
>> @@ -907,6 +1044,7 @@ static int mxf_read_header(AVFormatContext *s,
>> AVFormatParameters *ap)
>>  {
>>      MXFContext *mxf = s->priv_data;
>>      KLVPacket klv;
>> +    int64_t file_size = avio_size(s->pb);
>>
>>      if (!mxf_read_sync(s->pb, mxf_header_partition_pack_key, 14)) {
>>          av_log(s, AV_LOG_ERROR, "could not find header partition pack
>> key\n");
>> @@ -914,18 +1052,24 @@ static int mxf_read_header(AVFormatContext *s,
>> AVFormatParameters *ap)
>>      }
>>      avio_seek(s->pb, -14, SEEK_CUR);
>>      mxf->fc = s;
>> -    while (!url_feof(s->pb)) {
>> +    while (avio_tell(s->pb) < file_size) {
> 
> !s->pb->eof_reached?
> 
>>          const MXFMetadataReadTableEntry *metadata;
>>
>>          if (klv_read_packet(&klv, s->pb) < 0)
>>              return -1;
>>          PRINT_KEY(s, "read header", klv.key);
>>          av_dlog(s, "size %lld offset %#llx\n", klv.length, klv.offset);
>> +
>>          if (IS_KLV_KEY(klv.key, mxf_encrypted_triplet_key) ||
>> -            IS_KLV_KEY(klv.key, mxf_essence_element_key)) {
>> -            /* FIXME avoid seek */
>> -            avio_seek(s->pb, klv.offset, SEEK_SET);
>> -            break;
>> +            IS_KLV_KEY(klv.key, mxf_essence_element_key) ||
>> +            IS_KLV_KEY(klv.key, mxf_system_item_key)) {
>> +            if (!mxf->essence_offset) {
>> +                mxf->essence_offset = klv.offset;
>> +                if (IS_KLV_KEY(klv.key, mxf_system_item_key))
>> +                    mxf->system_item = 1;
>> +            }
>> +            avio_skip(s->pb, klv.length);
>> +            continue;
> 
> This is the part that'll be quite slow.
> 
>>          }
>>
>>          for (metadata = mxf_metadata_read_table; metadata->read;
>> metadata++) {
>> @@ -945,13 +1089,15 @@ static int mxf_read_header(AVFormatContext *s,
>> AVFormatParameters *ap)
>>          if (!metadata->read)
>>              avio_skip(s->pb, klv.length);
>>      }
>> +    /* FIXME avoid seek */
>> +    avio_seek(s->pb, mxf->essence_offset, SEEK_SET);
> 
> You might want to account for not finding any essence.
> 
> Anyway, hopefully the attached patches prove useful. The first one could
> probably be applied as-is. Baptiste?
> 
> /Tomas
> 
> 
> 0001-Parse-MXF-partitions.patch
> 
> 
> From d911eeb76e244f4b5d45c0a5e0a76a2bc5dc00c1 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <tomas.hardin at codemill.se>
> Date: Mon, 7 Feb 2011 16:07:41 +0100
> Subject: [PATCH 1/2] Parse MXF partitions
> 
> ---
>  libavformat/mxfdec.c |  104 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 103 insertions(+), 1 deletions(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 6b44b8f..61820f4 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -50,6 +50,27 @@
>  #include "avformat.h"
>  #include "mxf.h"
>  
> +typedef enum {
> +    Header,
> +    BodyPartition,
> +    Footer
> +} MXFPartitionType;
> +
> +typedef enum {
> +    OPAtom,
> +    OP1a,
> +} MXFOP;
> +
> +typedef struct {
> +    int closed;
> +    int complete;
> +    MXFPartitionType type;
> +    uint64_t previous_partition;
> +    uint64_t footer_partition;
> +    int index_sid;
> +    int body_sid;
> +} MXFPartition;
> +
>  typedef struct {
>      UID uid;
>      enum MXFMetadataSetType type;
> @@ -125,6 +146,9 @@ typedef struct {
>  } MXFMetadataSet;
>  
>  typedef struct {
> +    MXFPartition **partitions;
> +    unsigned partitions_count;

Any reason you don't use MXFPartition *partitions ?
It avoids malloc for each partition.

> [...]
>
> +
> +    /* consider both footers to be closed (there is only Footer and CompleteFooter) */
> +    partition->closed = partition->type == Footer || !(uid[14] & 1);

Do we really need to workaround here ?

> [...]
>
> +
> +    /* only grab OP from the header */
> +    if (partition->type != Header)
> +        return 0;

Why only header ? :)

> [...]
>
> @@ -843,6 +931,16 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
>  
>  static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
>      { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x05,0x01,0x00 }, mxf_read_primer_pack },
> +    { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,0x00 }, mxf_read_partition_pack },
> +    { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x02,0x00 }, mxf_read_partition_pack },
> +    { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x03,0x00 }, mxf_read_partition_pack },
> +    { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x04,0x00 }, mxf_read_partition_pack },
> +    { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x03,0x01,0x00 }, mxf_read_partition_pack },
> +    { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x03,0x02,0x00 }, mxf_read_partition_pack },
> +    { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x03,0x03,0x00 }, mxf_read_partition_pack },
> +    { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x03,0x04,0x00 }, mxf_read_partition_pack },
> +    { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x04,0x02,0x00 }, mxf_read_partition_pack },
> +    { { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x04,0x04,0x00 }, mxf_read_partition_pack },

Do we need to parse non complete partitions ?
Does it happen in the wild ? :/

Anyway, patch ok, nice work :)

I'll review the other patch later.

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org


More information about the ffmpeg-devel mailing list