[FFmpeg-soc] [Patch]Mxf muxer 0.0.1

zhentan feng spyfeng at gmail.com
Thu Aug 14 05:41:01 CEST 2008


Hi

2008/8/14 Michael Niedermayer <michaelni at gmx.at>

> On Wed, Aug 13, 2008 at 03:47:15PM +0800, zhentan feng wrote:
> > Hi,
> >
> > after changing the code in svn soc according to former reviews,
> > here is the diff between svn soc and ffmpeg trunk.
> >
> > mainly do this works:
> > 1) add mxfenc.c to impliment MXF simple muxer
> > 2) extract common code from mxfenc.c and mxfdec.c into mxf.h
> >
> > --
> > Best wishes~
>
> > Index: libavformat/mxfenc.c
> > ===================================================================
> > --- libavformat/mxfenc.c      (revision 0)
> > +++ libavformat/mxfenc.c      (revision 0)
> > @@ -0,0 +1,1058 @@
> > +/*
> > + * MXF muxer
> > + * Copyright (c) 2008 GUCAS, Zhentan Feng<spyfeng at gmail dot com>
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> > + */
> > +
> > +/*
> > + * References
> > + * SMPTE 336M KLV Data Encoding Protocol Using Key-Length-Value
> > + * SMPTE 377M MXF File Format Specifications
> > + * SMPTE 379M MXF Generic Container
> > + * SMPTE 381M Mapping MPEG Streams into the MXF Generic Container
> > + * SMPTE RP210: SMPTE Metadata Dictionary
> > + * SMPTE RP224: Registry of SMPTE Universal Labels
> > + */
> > +
>
> ok
> (this part and all other ok-ed parts can be commited
>  that way the patch will become smaller and easier to review)
>
I diff the files with FFmpeg trunk.
It seems that I can not commit to FFmpeg repo, but I'll improved it in soc
svn.


>
>
> > +#define DEBUG
>
> i don think that should be in the mxf muxer in the main tree
>
>
> [...]
>
> > +
> > +typedef struct {
> > +    UID uid;
> > +    enum CodecID type;
> > +} MXFEssenceElementKey;
>
> duplicate of MXFCodecUL
>

there are some difference with MXFCodecUL.
MXFCodecUL having matching len field for demuxer, because demuxer should
choose CodecID by mathcing Len,
when muxing, we do not need matching len, but we should choose uls by
CodecID.


>
> [...]
> > +static const MXFCodecUL mxf_essence_container_uls[] = {
> > +    // picture essence container
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x02,0x0D,0x01,0x03,0x01,0x02,0x04,0x60,0x01
> }, 14, CODEC_ID_MPEG2VIDEO }, /* MPEG-ES Frame wrapped */
> > +//    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x41,0x01
> }, 14,    CODEC_ID_DVVIDEO }, /* DV 625 25mbps */
> > +    // audio essence conatiner uls
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x06,0x01,0x00
> }, 14, CODEC_ID_PCM_S16LE }, /* BWF Frame wrapped */
> > +//    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x02,0x0D,0x01,0x03,0x01,0x02,0x04,0x40,0x01
> }, 14,       CODEC_ID_MP2 }, /* MPEG-ES Frame wrapped, 0x40 ??? stream id */
> > +//    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x01,0x01,0x01
> }, 14, CODEC_ID_PCM_S16LE }, /* D-10 Mapping 50Mbps PAL Extended Template */
> > +    { {
> 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> },  0,      CODEC_ID_NONE },
> > +};
>
> duplicate
>

I tried to extract this code ,however, we muxing by CodecID type to get UL,
but one CodecID have different ULs, such as CODEC_ID_PCM_S16LE.

so when checking each item in mxf_essence_container_uls for streams to build
essence container references, we can not decide which ul to use only by
CodecID type.
But we actually know which type we should mux, so just write what will be
muxed type in the table.

on the other side, demuxer can get the correct CodecID type by uls, uls are
unique.


>
>
> > +
> > +/**
> > + * SMPTE RP210 http://www.smpte-ra.org/mdd/index.html
> > + */
> > +static const MXFLocalTagPair mxf_local_tag_batch[] = {
> > +    // preface set
> > +    { 0x3C0A,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x01,0x01,0x15,0x02,0x00,0x00,0x00,0x00}},
> /* Instance UID */
> > +    { 0x3B02,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x07,0x02,0x01,0x10,0x02,0x04,0x00,0x00}},
> /* Last Modified Date */
> > +    { 0x3B05,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x03,0x01,0x02,0x01,0x05,0x00,0x00,0x00}},
> /* Version */
> > +    { 0x3B06,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x06,0x04,0x00,0x00}},
> /* Identifications reference */
> > +    { 0x3B03,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x02,0x01,0x00,0x00}},
> /* Content Storage reference */
> > +    { 0x3B09,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x01,0x02,0x02,0x03,0x00,0x00,0x00,0x00}},
> /* Operational Pattern UL */
> > +    { 0x3B0A,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x01,0x02,0x02,0x10,0x02,0x01,0x00,0x00}},
> /* Essence Containers UL batch */
> > +    { 0x3B0B,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x01,0x02,0x02,0x10,0x02,0x02,0x00,0x00}},
> /* DM Schemes UL batch */
> > +    // Identification
> > +    { 0x3C09,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x01,0x00,0x00,0x00}},
> /* This Generation UID */
> > +    { 0x3C01,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x02,0x01,0x00,0x00}},
> /* Company Name */
> > +    { 0x3C02,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x03,0x01,0x00,0x00}},
> /* Product Name */
> > +    { 0x3C04,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x04,0x00,0x00,0x00}},
> /* Version String */
> > +    { 0x3C05,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x07,0x00,0x00,0x00}},
> /* Product ID */
> > +    { 0x3C06,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x07,0x02,0x01,0x10,0x02,0x03,0x00,0x00}},
> /* Modification Date */
> > +    // Content Storage
> > +    { 0x1901,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x05,0x01,0x00,0x00}},
> /* Package strong reference batch */
> > +    // Essence Container Data
> > +    { 0x2701,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x06,0x01,0x00,0x00,0x00}},
> /* Linked Package UID */
> > +    { 0x3F07,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x04,0x01,0x03,0x04,0x04,0x00,0x00,0x00,0x00}},
> /* BodySID */
> > +    // Package
> > +    { 0x4401,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x01,0x01,0x15,0x10,0x00,0x00,0x00,0x00}},
> /* Package UID */
> > +    { 0x4405,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x07,0x02,0x01,0x10,0x01,0x03,0x00,0x00}},
> /* Package Creation Date */
> > +    { 0x4404,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x07,0x02,0x01,0x10,0x02,0x05,0x00,0x00}},
> /* Package Modified Date */
> > +    { 0x4403,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x06,0x05,0x00,0x00}},
> /* Tracks Strong reference array */
> > +    { 0x4701,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x02,0x03,0x00,0x00}},
> /* Descriptor */
> > +    // Track
> > +    { 0x4801,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x01,0x07,0x01,0x01,0x00,0x00,0x00,0x00}},
> /* Track ID */
> > +    { 0x4804,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x01,0x03,0x00,0x00}},
> /* Track Numberr */
> > +    { 0x4B01,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x30,0x04,0x05,0x00,0x00,0x00,0x00}},
> /* Edit Rate */
> > +    { 0x4B02,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x07,0x02,0x01,0x03,0x01,0x03,0x00,0x00}},
> /* Origin */
> > +    { 0x4803,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x02,0x04,0x00,0x00}},
> /* Sequence reference */
> > +    // Sequence
> > +    { 0x0201,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x07,0x01,0x00,0x00,0x00,0x00,0x00}},
> /* Data Definition UL */
> > +    { 0x0202,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x07,0x02,0x02,0x01,0x01,0x03,0x00,0x00}},
> /* Duration */
> > +    { 0x1001,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x06,0x09,0x00,0x00}},
> /* Structural Components reference array */
> > +    // Source Clip
> > +    { 0x1201,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x07,0x02,0x01,0x03,0x01,0x0A,0x00,0x00}},
> /* Start position */
> > +    { 0x1101,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x03,0x01,0x00,0x00,0x00}},
> /* SourcePackageID */
> > +    { 0x1102,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x03,0x02,0x00,0x00,0x00}},
> /* SourceTrackID */
> > +    // file descriptor
> > +    { 0x3F01,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x04,0x06,0x01,0x01,0x04,0x06,0x0B,0x00,0x00}},
> /* sub descriptor uid*/
> > +    { 0x3006,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x06,0x01,0x01,0x03,0x05,0x00,0x00,0x00}},
> /* Linked Track ID */
> > +    { 0x3001,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x06,0x01,0x01,0x00,0x00,0x00,0x00}},
> /* SampleRate */
> > +    { 0x3004,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x01,0x02,0x00,0x00}},
> /* essence container ul */
> > +    // generic picture eseence descriptor
>
> > +    { 0x3203,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x02,0x02,0x00,0x00,0x00}},
> /* stored width */
> > +    { 0x3202,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x02,0x01,0x00,0x00,0x00}},
> /* stored heigth */
> > +    { 0x320E,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x00,0x00,0x00}},
> /* aspect ratio*/
>
> i dont know mxf but what do these mean?
> the aspect ratio is not stored anywhere ...
>
>
> > +    { 0x3201,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x01,0x06,0x01,0x00,0x00,0x00,0x00}},
> /* picture essence coding*/
> > +    // generic sound essence descriptor
> > +    { 0x3D03,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x04,0x02,0x03,0x01,0x01,0x01,0x00,0x00}},
> /* audio sampling rate */
> > +    { 0x3D07,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x04,0x02,0x01,0x01,0x04,0x00,0x00,0x00}},
> /* channel count */
> > +    { 0x3D01,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x04,0x04,0x02,0x03,0x03,0x04,0x00,0x00,0x00}},
> /* quantization bits */
> > +    { 0x3D06,
> {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x01,0x06,0x01,0x00,0x00,0x00,0x00}},
> /* sound essence compression */
> > +};
>
> > +
> > +static void mxf_generate_uuid(AVFormatContext *s, UID uuid)
> > +{
> > +    MXFContext *mxf = s->priv_data;
> > +    int i;
> > +
> > +    for (i = 0; i < 16; i++) {
> > +        mxf->random_state= mxf->random_state*1664525+10139042;
> > +        uuid[i]= mxf->random_state>>24;
> > +    }
> > +    // the 7th byte is version according to ISO 11578
> > +    uuid[6] &= 0x0f;
> > +    uuid[6] |= 0x40;
> > +
> > +    // the 8th byte is variant for current use according to ISO 11578
> > +    uuid[8] &= 0x3f;
> > +    uuid[8] |= 0x80;
> > +}
> > +
> > +static void mxf_generate_umid(AVFormatContext *s, UMID umid)
> > +{
> > +    memcpy(umid, umid_base, 16);
> > +    mxf_generate_uuid(s, umid + 16);
> > +}
> > +
> > +static int mxf_generate_reference(AVFormatContext *s, UID **refs, int
> ref_count)
> > +{
> > +    int i;
> > +    UID *p;
> > +    *refs = av_mallocz(ref_count * sizeof(UID));
> > +    if (!*refs)
> > +        return AVERROR(ENOMEM);
> > +    p = *refs;
> > +    for (i = 0; i < ref_count; i++) {
> > +        mxf_generate_uuid(s, *p);
> > +        p ++;
> > +    }
> > +    return 0;
> > +}
> > +
>
> I think uid values should be made systeatically so they do not need to be
> stored. Currently weakly random UIDs are generated, stored in the context
> and then used, it would be simpler to just store systematic ones like
> constant or constant + stream_number.
>

there are many references and the number of references are also different in
metadata sets, IMHO this is more clear than writing them into context.
though it is a little waste of memory, unacceptable?


>
>
> > +static int klv_encode_ber_length(ByteIOContext *pb, uint64_t len)
> > +{
> > +    // Determine the best BER size
> > +    int size = 0;
> > +    uint64_t tmp = len;
> > +    if (len < 128) {
> > +        //short form
> > +        put_byte(pb, len);
> > +        return 1;
> > +    }
> > +
> > +    size = (av_log2(tmp) >> 3) + 1;
>
> size = 0 is unneeded
> tmp is redundant
>
>
> [...]
> > +static const MXFCodecUL *mxf_get_essence_container_ul(enum CodecID type)
> > +{
> > +    const MXFCodecUL *uls = mxf_essence_container_uls;
> > +    while (uls->id != CODEC_ID_NONE) {
> > +        if (uls->id == type)
> > +            break;
> > +        uls++;
> > +    }
> > +    return uls;
> > +}
>
> ok
>
>
> [...]
> > +static void mxf_write_local_tag(ByteIOContext *pb, int value_size, int
> tag)
> > +{
> > +    put_be16(pb, tag);
> > +    put_be16(pb, value_size);
> > +}
>
> ok
>
>
> [...]
> > +static void mxf_free(AVFormatContext *s)
> > +{
> > +    MXFContext *mxf = s->priv_data;
> > +    AVStream *st;
> > +    int i;
> > +
> > +    av_freep(&mxf->reference.identification);
> > +    av_freep(mxf->reference.package);
> > +    av_freep(&mxf->reference.package);
> > +    av_freep(&mxf->reference.content_storage);
> > +    for (i = 0; i < s->nb_streams; i++) {
> > +        st = s->streams[i];
> > +        av_freep(&st->priv_data);
> > +    }
> > +    av_freep(mxf->reference.sub_desc);
> > +    av_freep(&mxf->reference.sub_desc);
> > +    av_freep(&mxf->reference.mul_desc);
> > +    av_freep(&mxf->essence_container_uls);
> > +}
>
> ok
>
>
> > +
> > +static const MXFDataDefinitionUL *mxf_get_data_definition_ul(enum
> CodecType type)
> > +{
> > +    const MXFDataDefinitionUL *uls = mxf_data_definition_uls;
> > +    while (uls->type != CODEC_TYPE_DATA) {
> > +        if (type == uls->type)
> > +            break;
> > +        uls ++;
> > +    }
> > +    return uls;
> > +}
>
> ok
>
> [...]
> > +static int mxf_write_wav_desc(AVFormatContext *s, const
> MXFDescriptorWriteTableEntry *desc_tbl, int stream_index)
> > +{
> > +    MXFContext *mxf = s->priv_data;
> > +    MXFReferenceContext *refs = &mxf->reference;
> > +    ByteIOContext *pb = s->pb;
> > +    AVStream *st;
> > +    const MXFCodecUL *codec_ul = NULL;
> > +
> > +    st = s->streams[stream_index];
> > +
> > +    put_buffer(pb, desc_tbl->key, 16);
> > +    klv_encode_ber_length(pb, 96);
> > +
> > +    mxf_write_local_tag(pb, 16, 0x3C0A);
> > +    put_buffer(pb, (*refs->sub_desc)[stream_index], 16);
> > +
> > +    mxf_write_local_tag(pb, 4, 0x3006);
> > +    put_be32(pb, stream_index);
> > +#ifdef DEBUG
> > +    PRINT_KEY(s, "wav desc uid", (*refs->track)[stream_index]);
> > +#endif
> > +    codec_ul = mxf_get_essence_container_ul(st->codec->codec_id);
> > +    mxf_write_local_tag(pb, 16, 0x3004);
> > +    put_buffer(pb, codec_ul->uid, 16);
>
> duplicate
>
>
> [...]
> > +            for (desc = mxf_descriptor_write_table; desc->write; desc++)
> {
> > +                if (s->streams[i]->codec->codec_id == desc->type) {
> > +                    ret = desc->write(s, desc, i);
> > +                    if ( ret < 0) {
> > +                        av_log(s, AV_LOG_ERROR, "error writing
> descriptor\n");
> > +                        goto fail;
> > +                    }
> > +                    break;
> > +                }
> > +            }
>
> The table of function pointers seems overkill for 2 if() func()


we can add some other descriptors in the table later.
this will be clear like demuxer read table.

>
>
>
> [...]
> > +static int mxf_add_essence_container_ul(MXFContext *mxf, const
> MXFCodecUL *codec_ul)
> > +{
> > +    mxf->essence_container_uls = av_realloc(mxf->essence_container_uls,
> (mxf->essence_container_count + 1) * 16);
> > +    if (!mxf->essence_container_uls)
> > +        return AVERROR(ENOMEM);
> > +    memcpy(mxf->essence_container_uls[mxf->essence_container_count],
> codec_ul->uid, 16);
> > +    mxf->essence_container_count++;
> > +    return mxf->essence_container_count;
> > +}
> > +
> > +static int mxf_build_essence_container_refs(AVFormatContext *s)
> > +{
> > +    MXFContext *mxf = s->priv_data;
> > +    AVStream *st;
> > +    int i;
> > +    const MXFCodecUL *codec_ul = NULL;
> > +
> > +    for (codec_ul = mxf_essence_container_uls; codec_ul->id; codec_ul++)
> {
> > +        for (i = 0; i < s->nb_streams; i++) {
> > +            st = s->streams[i];
> > +            if (st->codec->codec_id == codec_ul->id) {
> > +                if (mxf_add_essence_container_ul(mxf, codec_ul) < 0 )
> > +                    return -1;
> > +                break;
> > +            }
> > +        }
> > +    }
> > +    return 0;
> > +}
>
> There is no need to build these in memory with realloc they can each be
> stored immedeatly as they are found
>

there are at least three times to write the references in
mxf_write_preface_set(), mxf_write_header/footer_partition().
so, if do not stroed in mem, we should check it for each time.

>
>
> > +
>
> > +static void mxf_write_partition(AVFormatContext *s, int64_t
> this_partition, int bodysid, const uint8_t *key)
> > +{
> > +    MXFContext *mxf = s->priv_data;
> > +    ByteIOContext *pb = s->pb;
> > +#ifdef DEBUG
> > +    int i;
> > +#endif
> > +    // write klv
> > +    put_buffer(pb, key, 16);
> > +    klv_encode_ber_length(pb, 88 + 16 * mxf->essence_container_count);
> > +
> > +    // write partition value
> > +    put_be16(pb, 1); // majorVersion
> > +    put_be16(pb, 2); // minorVersion
> > +    put_be32(pb, 1); // kagSize
> > +
> > +    put_be64(pb, this_partition); // thisPartition
> > +    put_be64(pb, 0); // previousPartition
> > +
>
> > +    // set offset
> > +    if (!this_partition)
> > +        mxf->header_footer_partition_offset = url_ftell(pb);
>
> i think "this_partition" is not a good name for the current byte offset
>
yes, it is a little not clear.
but "this partition" is field name according to s377M.


>
>
> > +    put_be64(pb, this_partition); // footerPartition,update later
> > +
> > +    // set offset
> > +    if (!this_partition)
> > +        mxf->header_byte_count_offset = url_ftell(pb);
> > +    put_be64(pb, 0); // headerByteCount, update later
> > +
>
> > +    // no indexTable
> > +    put_be64(pb, 0); // indexByteCount
> > +    put_be32(pb, 0); // indexSID
> > +    put_be64(pb, 0); // bodyOffset
>
> why? Isnt an index required for seeking?


we generate mxf file use op1a, and index table is optional for op1a.

>
>
>
> [...]
> > +static int mux_write_header(AVFormatContext *s)
> > +{
> > +    MXFContext *mxf = s->priv_data;
> > +    ByteIOContext *pb = s->pb;
> > +    int64_t header_metadata_start;
> > +
> > +    // calculate the numner of essence container type
>
> numner?
>
>
> [...]
> > +static int mux_write_packet(AVFormatContext *s, AVPacket *pkt)
> > +{
> > +    ByteIOContext *pb = s->pb;
> > +    AVStream *st = s->streams[pkt->stream_index];
> > +    MXFStreamContext *sc = st->priv_data;
> > +
> > +    put_buffer(pb, sc->track_essence_element_key, 16); // write key
> > +    klv_encode_ber_length(pb, pkt->size); // write length
> > +    put_buffer(pb, pkt->data, pkt->size); // write value
> > +
> > +    put_flush_packet(pb);
> > +    return 0;
> > +}
>
> ok
>
>
> > +
> > +static int mxf_update_header_partition(AVFormatContext *s, int64_t
> footer_partition_offset)
> > +{
> > +    MXFContext *mxf = s->priv_data;
> > +    ByteIOContext *pb = s->pb;
> > +
> > +    url_fseek(pb, mxf->header_byte_count_offset, SEEK_SET);
> > +    put_be64(pb, mxf->header_byte_count);
> > +    put_flush_packet(pb);
>
> this can be written during header writing already
>
>
> > +
> > +    url_fseek(pb, mxf->header_footer_partition_offset, SEEK_SET);
> > +    put_be64(pb, footer_partition_offset);
> > +    put_flush_packet(pb);
> > +    return 0;
> > +}
>
> this is missing a check for url_is_streamed()
>
>
> [...]
> > +AVOutputFormat mxf_muxer = {
> > +    "mxf",
> > +    NULL_IF_CONFIG_SMALL("Material eXchange Format"),
> > +    NULL,
> > +    "mxf",
> > +    sizeof(MXFContext),
> > +    CODEC_ID_PCM_S16LE,
> > +    CODEC_ID_MPEG2VIDEO,
> > +    mux_write_header,
> > +    mux_write_packet,
> > +    mux_write_footer,
> > +};
>
> ok
>
>
> > +
> > +
> > Index: libavformat/mxfdec.c
> > ===================================================================
> > --- libavformat/mxfdec.c      (revision 14722)
> > +++ libavformat/mxfdec.c      (working copy)
> > @@ -46,24 +46,8 @@
> >  //#define DEBUG
> >
> >  #include "libavutil/aes.h"
> > -#include "libavcodec/bytestream.h"
> > -#include "avformat.h"
> > +#include "mxf.h"
>
> spliting mxf.c into mxfdec.c/mxf.c should be a seperate patch
>
> [...]
> > Index: libavformat/mxf.h
> > ===================================================================
> > --- libavformat/mxf.h (revision 0)
> > +++ libavformat/mxf.h (revision 0)
> [...]
> > +/**
> > + * SMPTE RP224 http://www.smpte-ra.org/mdd/index.html
> > + */
> > +static const MXFDataDefinitionUL mxf_data_definition_uls[] = {
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x01,0x03,0x02,0x02,0x01,0x00,0x00,0x00
> }, CODEC_TYPE_VIDEO },
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x01,0x03,0x02,0x02,0x02,0x00,0x00,0x00
> }, CODEC_TYPE_AUDIO },
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x05,0x01,0x03,0x02,0x02,0x02,0x02,0x00,0x00
> }, CODEC_TYPE_AUDIO },
> > +    { {
> 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> },  CODEC_TYPE_DATA },
> > +};
> > +
> > +static const MXFCodecUL mxf_codec_uls[] = {
> > +    /* PictureEssenceCoding */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x01,0x11,0x00
> }, 14, CODEC_ID_MPEG2VIDEO }, /* MP at ML Long GoP */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x01,0x02,0x01,0x01
> }, 14, CODEC_ID_MPEG2VIDEO }, /* D-10 50Mbps PAL */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x03,0x03,0x00
> }, 14, CODEC_ID_MPEG2VIDEO }, /* MP at HL Long GoP */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x04,0x02,0x00
> }, 14, CODEC_ID_MPEG2VIDEO }, /* 422P at HL I-Frame */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x20,0x02,0x03
> }, 14,      CODEC_ID_MPEG4 }, /* XDCAM proxy_pal030926.mxf */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x01,0x02,0x00
> }, 13,    CODEC_ID_DVVIDEO }, /* DV25 IEC PAL */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x07,0x04,0x01,0x02,0x02,0x03,0x01,0x01,0x00
> }, 14,   CODEC_ID_JPEG2000 }, /* JPEG2000 Codestream */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x01,0x7F,0x00,0x00,0x00
> }, 13,   CODEC_ID_RAWVIDEO }, /* Uncompressed */
> > +    /* SoundEssenceCompression */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x01,0x00,0x00,0x00,0x00
> }, 13,  CODEC_ID_PCM_S16LE }, /* Uncompressed */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x01,0x7F,0x00,0x00,0x00
> }, 13,  CODEC_ID_PCM_S16LE },
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x07,0x04,0x02,0x02,0x01,0x7E,0x00,0x00,0x00
> }, 13,  CODEC_ID_PCM_S16BE }, /* From Omneon MXF file */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x04,0x04,0x02,0x02,0x02,0x03,0x01,0x01,0x00
> }, 15,   CODEC_ID_PCM_ALAW }, /* XDCAM Proxy C0023S01.mxf */
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x01,0x00
> }, 15,        CODEC_ID_AC3 },
> > +    { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x05,0x00
> }, 15,        CODEC_ID_MP2 }, /* MP2 or MP3 */
> > +  //{ {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x1C,0x00
> }, 15,    CODEC_ID_DOLBY_E }, /* Dolby-E */
> > +    { {
> 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> },  0,       CODEC_ID_NONE },
> > +};
>
> putting tables in a header would make them duplicated in each file that
> includes that header


yes, but they are actually be used  by mxfenc.c and mxfdec.c

thanks for your reviews.
I'll modified them asap.
-- 
Best wishes~



More information about the FFmpeg-soc mailing list