[FFmpeg-soc] MXF muxer version 0.0.4

[Michael Niedermayer]

On Sat, Aug 23, 2008 at 06:23:16PM +0200, Baptiste Coudurier wrote:
> Michael Niedermayer wrote:
> > [...]
> >
> >>  static void mxf_update_header_partition(AVFormatContext *s, int64_t footer_partition_offset)
> >>  {
> >>      MXFContext *mxf = s->priv_data;
> >> Index: mxf.h
> >> ===================================================================
> >> --- mxf.h	(revision 14903)
> >> +++ mxf.h	(working copy)
> >> @@ -41,6 +41,7 @@
> >>      Identification,
> >>      ContentStorage,
> >>      SubDescriptor,
> >> +    TypeBottom,// add metadata type before this
> >>  };
> >>  
> >>  typedef struct {
> > 
> > ok
> > 
> > 
> 
> Why do we need this ?

TypeBottom? Iam not happy about it either, i just gave up after
"Sequence" and 0xf0 being used in its place. It didnt appear important
enough to reject the code and i had some communication problems with zhentan


> 
> I don't understand why do we need this for uuid genereration ?
> 
> My idea is set a MXFContext->cur_uid to a static start value, then
> increment this value for each uid needed.
> 

> Is there something wrong with this ? I don't see how this mess (if
> MaterialPackage etc ...) is needed.

that if() can surely be avoided ...


> 
> UID is just a way to reference unique metadata in mxf file, enum seems
> not needed, streams[0] and streams[1] does not necessearly need to have
> number in sequence.

no, but if they do not then you will have to store many UUIDs, thats the way
it was originally and it was a huge mess

struct{
    thisUUID
    thatUUID
    moreUUID[10]
    *evenMoreUUID;
    evenMoreUUID_count;
}

and then
1. init
    init_uuid(s->thisUUID);
    init_uuid(s->thatUUID);
    for(i=0; i<10; i++)
        init_uuid(s->moreUUID[i]);
    s->evenMoreUUID= malloc(s->evenMoreUUID_count * sizeof)
    for(i=0; i<s->evenMoreUUID_count; i++)
        init_uuid(s->evenMoreUUID[i]);
2. write references (and the same again for the actual references parts)
    write_uuid(s->thisUUID);
    write_uuid(s->thatUUID);
    for(i=0; i<10; i++)
        write_uuid(s->moreUUID[i]);
    for(i=0; i<s->evenMoreUUID_count; i++)
        write_uuid(s->evenMoreUUID[i]);
3. free
    av_freep(&s->evenMoreUUID);

that is vs. the current:

1. init
    nothing

2. write reference 
    write_uuid(THIS_UUID, 0);
    write_uuid(THAT_UUID, 1);
    for(i=0; i<10; i++)
        write_uuid(MORE_UUID, i);
    for(i=0; i<s->evenMoreUUID_count; i++)
        write_uuid(EVEN_MORE_UUID[i], i);

3. free
    nothing


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20080823/e4efa2e9/attachment.pgp>