[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