[FFmpeg-soc] mxfenc.c version 0.0.4

Michael Niedermayer michaelni at gmx.at
Wed Jul 30 16:39:41 CEST 2008


On Wed, Jul 30, 2008 at 01:13:50AM +0800, zhentan feng wrote:
> Hi,
> 
> mxfenc.c is version 0.0.4 for now, the attachement is source file.
> There are some codes duplication with mxf.c, once mxfenc.c works correctly,
> I'll factorize them into common header file.
> 
> I run the primary test as this:
> 1)./output_example test.mpg
> 2)./ffmpeg -i test.mpg test.mxf
> outputs like this:
> 
> [mpeg @ 0x8410560]Invalid timestamps stream=0, pts=0, dts=8589930992,
> size=2002
> Input #0, mpeg, from 'test.mpg':
>   Duration: 00:00:05.00, start: 0.000000, bitrate: 648 kb/s
>     Stream #0.0[0x1e0], 1/90000: Video: mpeg1video, yuv420p, 352x288 [PAR
> 1:1 DAR 11:9], 1/25, 104857 kb/s, 25.00 tb(r)
>     Stream #0.1[0x1c0], 1/90000: Audio: mp2, 44100 Hz, stereo, 64 kb/s
> frame=  126 fps= 67 q=18.6 Lsize=    1154kB time=5.00 bitrate=1890.3kbits/s
> video:281kB audio:864kB global headers:0kB muxing overhead 0.805797%
> 
> 
>  though it works and ./ffplay can play the mxf file, the display effect of
> ./ffplay test.mxf are not as good as ./ffplay test.mpg.
> it seems that some fields are improper set.
> any ideas?

use 
./ffmpeg -i test.mpg -vcodec copy test.mxf
or
./ffmpeg -i test.mpg -vqscale 1 test.mxf

[...]
> /* SMPTE RP210 http://www.smpte-ra.org/mdd/index.html */
> static const MXFLocalTagPair mxf_local_tag_batch[] = {

The comment should be doxygen compatible


[...]
> static void mxf_generate_uuid(AVFormatContext *s, UID uuid)
> {
>     MXFContext *mxf = s->priv_data;
>     int rand_num, i;
> 
>     for (i = 0; i < 16; i++) {
>         rand_num = av_random(&mxf->random_state);
>         rand_num &= 0x00ff;
>         uuid[i] = rand_num;

you dont need the MT random number generator for this

mxf->random_state= mxf->random_state*1664525+1013904223
uuid[i]= mxf->random_state>>24
will work as well and is simpler
random_state being unsigned int


[...]
> 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);

*refs


>     p = *refs;
>     for (i = 0; i < ref_count; i++) {
>         mxf_generate_uuid(s, *p);
>         p ++;
>     }
>     return 0;
> }
> 
> 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

>         size = 1;
>         put_byte(pb, len);
>         return 1;

size is unused


>     }
> 
>     while (tmp) {
>         tmp >>= 8;
>         size ++;
>     }
> 
>     // long form
>     put_byte(pb, 0x80 + size);
>     while(size) {
>         size --;
>         put_byte(pb, len >> 8 * size & 0xff);
>     }
>     return size;

thats the same as return 0


[...]
> static void mxf_write_reference(ByteIOContext *pb, int ref_count, UID *value)
> {
>     put_be32(pb, ref_count);
>     put_be32(pb, 16);
>     put_buffer(pb, *value, 16 * ref_count);
> }

UID value
put_buffer(pb, value, sizof(UID) * ref_count


[...]
> 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;
> }

duplicate of mxf_get_essence_container_ul


[...]
> static int mxf_write_package(AVFormatContext *s, KLVPacket *klv, enum MXFMetadataSetType type)
> {
>     MXFContext *mxf = s->priv_data;
>     MXFReferenceContext *refs = mxf->reference;
>     ByteIOContext *pb = s->pb;
>     UMID umid;
>     int i;
> 
>     klv->key[13] = 0x01;
>     klv->key[14] = type == MaterialPackage ? 0x36 : 0x37;
>     klv->key[15] = 0x00;
> 
>     put_buffer(pb, klv->key, 16);
>     if (type == MaterialPackage)
>         klv_encode_ber_length(pb, 92 + 16 * s->nb_streams);
>     else
>         klv_encode_ber_length(pb, 112 + 16 * s->nb_streams); // 20 bytes length for descriptor reference
> 
>     // write uid
>     i = type == MaterialPackage ? 0 : 1;
>     mxf_write_local_tag(pb, 16, 0x3C0A);
>     put_buffer(pb, (*refs->package)[i], 16);
> #ifdef DEBUG
>     av_log(s,AV_LOG_DEBUG, "package type:%d\n", type);
>     PRINT_KEY(s, "package", klv->key);
>     PRINT_KEY(s, "package uid", (*refs->package)[i]);
>     PRINT_KEY(s, "package umid first part", umid);
>     PRINT_KEY(s, "package umid second part", umid + 16);
> #endif
> 
>     // write package umid
>     mxf_write_local_tag(pb, 32, 0x4401);
>     if (type == MaterialPackage) {
>         mxf_generate_umid(s, umid);
>         put_buffer(pb, umid, 32);
>     } else {
>         put_buffer(pb, mxf->top_src_package_uid, 32);
>     }
> 
>     // write create date
>     mxf_write_local_tag(pb, 8, 0x4405);
>     put_be64(pb, 0);
> 
>     // write modified date
>     mxf_write_local_tag(pb, 8, 0x4404);
>     put_be64(pb, 0);
> 
>     // write track refs
>     refs->track = av_mallocz(s->nb_streams * sizeof(*refs->track));
>     if (!refs->track)
>         return AVERROR(ENOMEM);
>     if (mxf_generate_reference(s, refs->track, s->nb_streams) < 0)
>         return -1;
>     mxf_write_local_tag(pb, s->nb_streams * 16 + 8, 0x4403);
>     mxf_write_reference(pb, s->nb_streams, *refs->track);
> 
>     // every track have 1 sequence and 1 structural componet, malloc memory for the refs pointer

>     refs->sequence = av_mallocz(s->nb_streams * sizeof(*refs->sequence));
>     if (!refs->sequence)
>         return AVERROR(ENOMEM);
>     refs->structural_component = av_mallocz(s->nb_streams * sizeof(*refs->structural_component));
>     if (!refs->structural_component)
>         return AVERROR(ENOMEM);
> 
>     // malloc memory for track number sign
>     if (type == SourcePackage) {

>         mxf->track_number_sign = av_mallocz(sizeof(mxf_essence_element_key)/sizeof(MXFEssenceElementKey));

whichever sizes these are, they are constant and thus the malloc isnt needed


>         if (!mxf->track_number_sign)
>             return AVERROR(ENOMEM);
>         // write multiple descriptor reference
>         if (mxf_generate_reference(s, &refs->mul_desc, 1) < 0)
>             return -1;
>         mxf_write_local_tag(pb, 16, 0x4701);
>         put_buffer(pb, *refs->mul_desc, 16);
>     }
> 
>     // malloc memory for essence element key of each track
>     mxf->track_essence_element_key = av_mallocz(s->nb_streams * sizeof(UID));

a single struct per stream and placed in AVFormatContext.streams[i].priv_data
seems to make more sense than dozends of malloced arrays


[...]
> static int mxf_build_structural_metadata(AVFormatContext *s, KLVPacket* klv, enum MXFMetadataSetType type)
> {
>     int i;
>     const MXFDescriptorWriteTableEntry *desc = NULL;
> 
>     if (mxf_write_package(s, klv, type) < 0)
>         return -1;
>     if (type == SourcePackage) {
>         if (mxf_write_multi_descriptor(s, klv) < 0)
>             return -1;
>     }
> 
>     for (i = 0;i < s->nb_streams; i++) {
>         if (mxf_write_track(s, klv, i, type) < 0)
>             return -1;
> 
>         if (mxf_write_sequence(s, klv, i) < 0)
>             return -1;
> 
>         if (mxf_write_structural_component(s, klv, i, type) < 0)
>             return -1;
> 
>         if (type == SourcePackage) {

>             for (desc = mxf_descriptor_read_table; desc->write; desc++) {
>                 if (s->streams[i]->codec->codec_id == desc->type) {

i think there are alraedy 2 functions that search for a specific descriptor


>                     int (*write)() = desc->write;
>                     if (write(s, desc, i) < 0) {

redundant variable


[...]
> static int mxf_build_essence_container_refs(AVFormatContext *s)
> {
>     MXFContext *mxf = s->priv_data;
>     AVStream *st;
>     int i;
>     const MXFCodecUL *codec_ul = NULL;
> 
>     for (i = 0; i < s->nb_streams; i++) {
>         st = s->streams[i];
>         switch (st->codec->codec_type) {
>         case CODEC_TYPE_VIDEO:
>             codec_ul = mxf_get_essence_container_ul(mxf_picture_essence_container_uls, st->codec->codec_id);
>             break;
>         case CODEC_TYPE_AUDIO:
>             codec_ul = mxf_get_essence_container_ul(mxf_sound_essence_container_uls, st->codec->codec_id);
>             break;
>         }
> 
>         if (codec_ul) {
>             if (mxf_add_essence_container_ul(mxf, codec_ul) < 0 )
>                 return -1;
>         } else
>             return -1;

assuming this code is correct, it would be simpler to iterate over all codecs
and check for each if it is used by any stream and if so add it.
Instead of iterating over all streams and then checkng if the codec has
already been added.

Also it seems to make no sense to split the video and audio 
"essence_container_uls" in 2 tables, the code would be simpler if it was one
table


[...]
> static int mux_write_header(AVFormatContext *s)
> {
>     MXFContext *mxf = s->priv_data;
>     ByteIOContext *pb = s->pb;
>     int64_t header_metadata_start;
> 
>     av_init_random(0xbeefdead, &mxf->random_state);
>     // intial MXFReferenceContext

>     mxf->reference = av_mallocz(sizeof(MXFReferenceContext));
>     if (!mxf->reference)
>         goto fail;

MXFReferenceContext could be part of MXFContext that would avoid the malloc.


> 
>     // mark the header start position, for some fields update later
>     mxf->header_start = url_ftell(pb);

this is 0 thus unneeded


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/20080730/ef3236cd/attachment.pgp>


More information about the FFmpeg-soc mailing list