[FFmpeg-soc] [Patch]Mxf muxer 0.0.1

zhentan feng spyfeng at gmail.com
Thu Aug 14 19:03:52 CEST 2008


Hi

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

> On Thu, Aug 14, 2008 at 11:41:01AM +0800, zhentan feng wrote:
> > 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.
>
> baptiste (or any othetr volunteer), can you commit the ok-ed parts?
>
>
> >
> >
> > >
> > >
> > > > +#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.
>
> the muxer can ignore len.
>
>
> >
> >
> > >
> > > [...]
> > > > +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.
>
> no reason not the share the tables, either CODEC_ID_PCM_S16LE must be
> special
> handled or the UL used for muxing can be the first CODEC_ID_PCM_S16LE in
> the
> table.
>

hmm, to get the ul when muxing, i think it is not enough only by CodecID
type.
Is it necessary to add some flag in the table( or get information from some
api in FFmpeg) to distinguish one CodecID type having different uls?


>
>
> >
> > 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 ...
>
> ping?
>
I made a mistake in mxfenc.c, i restored time_base, this is obviously wrong.

I meaned to store aspect ratio of the video.
Is AVCodecContext->sample_aspect_ratio correct?



>
>
>
> > >
> > >
> > > > +    { 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?
>
> write_uid(stream_num or codec_id or ...)
> ...
> write_uid(stream_num or codec_id or ...)
>
> vs.
> context->variable_UID= av_malloc(sizeof(UID*));
> for(i=0; i<stream_num; i++)
>    context->variable_UID[i]= av_malloc(sizeof(UID));
> for(i=0; i<stream_num; i++)
>    context->variable_UID[i]= generate_random_uid()
> put_buffer(context->variable_UID);
> ...
> put_buffer(context->variable_UID);
>
>
> My suggstion looks distinctly simpler
>
> I am a little confused about your suggstion :(

you know this is a reference chain,
for example, once  have generatee refs for tracks in package set,
when writing track set, it just read the uid by
MXFReferenceContext->track[i].

at this situation, we should generate the refs for tracks while wrting
package set, becasue some fields of package set are the value of the
references, so it can not be put into " for (i=0; i<stream_num; i++)" to
generate.

on the other hand, stream_num is equal to the number of track sets, so we
generate references for sequence set and structual componet set in "for
(i=0; i<stream_num)" while writing track set and sequence set.

MXFReferenceContext have all the references pointer.
MXFStreamContext all common value for each stream including the references
pointer.


>
> [...]
>
> > > > +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.
>
> yes
>
>
> >
> > >
> > >
> > > > +
> > >
> > > > +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.
>
> byte_position is a better name IMHO
>
>
>
> >
> >
> > >
> > >
> > > > +    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.
>
> That is not what i was asking. They surely are optional, but they are
> required to be able to seek in non cbr files. The spec also says:
>
> "Index Tables should be implemented wherever possible. They can be used to
> satisfy a number of the User
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>  Requirements, particularly those to do with the handling of partial files
> (for example reading part of a large file
>  from the middle of a data tape, indexed by the timeline). When Index
> Tables are used, they shall conform to this
>  specification.
> "
>
well, now the mxf file doesn't contain indext table, so it can not be
seeked.
however, mxfdec.c seems not read any values of index table, and
mxf_read_seek() doesn't use any information about of index table.
Sholud i must add index table?


>
>
>
> [...]
> > > [...]
> > > > 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
>
> just put them in mxf.c, remove static and a add a ff_ prefix.
>
>

other reviews which are not replied default as understand and will change
soon.
thanks very much.
-- 
Best wishes~



More information about the FFmpeg-soc mailing list