[FFmpeg-soc] MXF muxer version 0.0.2
Michael Niedermayer
michaelni at gmx.at
Tue Jul 22 01:38:05 CEST 2008
On Tue, Jul 22, 2008 at 01:19:57AM +0800, zhentan feng wrote:
> Hi,
> By now, the mxfenc.c works without exceptions, but there are still some
> bugs:
[...]
> typedef struct {
> UID key;
> offset_t offset;
> uint64_t length;
> } KLVPacket;
>
> typedef struct {
> UID uid;
> unsigned matching_len;
> enum CodecID id;
> } MXFCodecUL;
duplicate of the same named thing in mxf.c
shared stuff should be moved to common mxf.c / mxf.h and the demuxer
renamed to mxfdec.c
[...]
> /* complete key */
> static const uint8_t op1a_ul[] = { 0x06, 0x0e, 0x2b, 0x34, 0x04, 0x01, 0x01, 0x01, 0x0d, 0x01, 0x02, 0x01, 0x01, 0x01, 0x01, 0x00 };
comments should be doxygen compatible where appropriate
> static const uint8_t header_partition_key[] = { 0x06, 0x0e, 0x2b, 0x34, 0x02, 0x05, 0x01, 0x01, 0x0d, 0x01, 0x02, 0x01, 0x01, 0x02, 0x04, 0x00 }; // ClosedComplete
> static const uint8_t footer_partition_key[] = {0x06, 0x0e, 0x2b, 0x34, 0x02, 0x05, 0x01, 0x01, 0x0d, 0x01, 0x02, 0x01, 0x01, 0x04, 0x04, 0x00}; // ClosedComplete
> static const uint8_t primer_pack_key[] = { 0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x05,0x01,0x00 };
upper / lower case is inconsistantly used.
[...]
> /* 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 },
> };
duplicate relative to mxf.c
[...]
> 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 = rand_num & 0x00ff;
is there any reason why this i better than a
int random_state in the contzext and a
random_state++ ?
>
> // the 7th byte is version according to ISO 11578
> if (i == 6) {
> rand_num &= 0x0f;
> rand_num |= 0x40;
> }
>
> // the 8th byte is variant for current use according to ISO 11578
> if (i == 8) {
> rand_num &= 0x3f;
> rand_num |= 0x80;
> }
> uuid[i] = rand_num;
> }
uuid[i] = rand_num;
}
uuid[6] &= 0x0f;
uuid[6] |= 0x40;
...
> }
>
> 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 -1;
> p = *refs;
> for (i = 0; i < ref_count; i++) {
> mxf_generate_uuid(s, *p);
> p += 16;
> }
> p = 0;
> return 0;
this code looks strange
> }
>
> static int klv_encode_ber_length(ByteIOContext *pb, uint64_t len)
> {
> // Determine the best BER size
> int size = 0, i;
> uint64_t tmp = len;
> if (len < 128) {
> //short form
> size = 1;
> put_byte(pb, len);
> return size;
return 1
> }
>
> while (tmp) {
> tmp >>= 8;
> size ++;
> }
>
> // long form
> put_byte(pb, 0x80 + size);
> i = size;
> while(i) {
> put_byte(pb, len & 0xff);
> len >>= 8;
> i--;
> }
for(i=0; i<size; i++)
> return size;
> }
>
> static int mxf_write_primer_pack(AVFormatContext *s)
> {
> ByteIOContext *pb = s->pb;
> const MXFLocalTagPair *local_tag_batch;
> int i,local_tag_number = 0;
>
> local_tag_number = sizeof(mxf_local_tag_batch) / sizeof(MXFLocalTagPair);
>
> put_buffer(pb, primer_pack_key, 16);
> klv_encode_ber_length(pb, local_tag_number * 18 + 8);
>
> put_be32(pb, local_tag_number); // local_tag num
> put_be32(pb, 18); // item size, always 18 according to the specs
>
> for (local_tag_batch = mxf_local_tag_batch; i < local_tag_number; local_tag_batch++, i++) {
i is uninitalized and iam pretty sure your compiler told you about this
[...]
> static int 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);
> return 0;
> }
I think the argument should be UID value
and the function should be void
>
> static int utf8len(const uint8_t *b){
> int len=0;
> int val;
> while(*b){
> GET_UTF8(val, *b++, return -1;)
> len++;
> }
> return len;
> }
duplicate
[...]
> static const MXFDataDefinitionUL *mxf_get_data_definition_ul(const MXFDataDefinitionUL *uls, enum CodecType type)
> {
> while (uls->type != CODEC_TYPE_DATA) {
> if (type == uls->type)
> break;
> uls ++;
> }
> return uls;
> }
MXFDataDefinitionUL has always the same value, thus doesnt need to be
passed to the function
[...]
> static int mxf_write_identification(AVFormatContext *s, KLVPacket *klv)
> {
> MXFContext *mxf = s->priv_data;
> MXFReferenceContext *refs = mxf->reference;
> ByteIOContext *pb = s->pb;
> UID uid;
> int length, company_name_len, product_name_len, version_string_len;
>
> klv->key[13] = 0x01;
> klv->key[14] = 0x30;
> klv->key[15] = 0x00;
>
> put_buffer(pb, klv->key, 16);
>
> company_name_len = utf8len("FFmpeg") + 1;
> product_name_len = utf8len("OP1a Muxer") + 1;
> version_string_len = utf8len("version 0.0.1") + 1;
see LIBAVFORMAT_IDENT
[...]
> // malloc memory for track number sign
> if (type == SourcePackage) {
> mxf->track_number_sign = av_mallocz(sizeof(mxf_essence_element_key)/sizeof(MXFEssenceElementKey));
> if (!mxf->track_number_sign)
> return -1;
> }
>
> // malloc memory for essence element key of each track
> mxf->track_essence_element_key = av_mallocz(s->nb_streams * sizeof(UID));
> if (!mxf->track_essence_element_key)
> return -1;
memleak
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- 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/20080722/2bf10204/attachment.pgp>
More information about the FFmpeg-soc
mailing list