On Mon, 2014-10-27 at 17:21 +0000, Thomas Mundt wrote: > Hi, I´ve seen that there has been approach last month to implement AVC Intra > mxf muxing. I tested the patches, but it didn´t work with any of my samples. > Since there also has been discussions about the gpl restriction, I rewrote > the patch. I had some basics, because I had written a working patch for > myself some time ago, which was more of a hack and only worked with AVCI100 > 1080i50. > I hope this could be licenced to lgpl, because I got all labels from libmxf > and libbmx and only used code snippets from avcodec/h264_parser.c > To keep h264 parsing simple and fast, I used the framesize for selecting the > right Panasonic codec label. The framesize is fixed for Panasonic AVC Intra. > > This patch only supports AVCI50/100. But in all flavours, i.e. with no > SPS/PPS in header. > > http://pastebin.com/v7gF1vDq > > Thomas
Could you rewrite it so you don't mix functional changes with indentation changes? See mxf_write_mpegvideo_desc() > > + switch (pkt->size + extrasize) { > + case 116736: // AVCI50 720p60 > + sc->codec_ul = &mxf_h264_codec_uls[5]; > + break; > + case 140800: // AVCI50 720p50 > + sc->codec_ul = &mxf_h264_codec_uls[6]; > + break; The magic values here stink. You should stick them next to mxf_h264_codec_uls, perhaps using a struct like so: static const struct { UID uid; int packet_size; int profile; uint8_t interlaced; } mxf_h264_codec_uls[] = { {{ 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0a,0x04,0x01,0x02,0x02,0x01,0x32,0x20,0x01 }, 0, 110, 0}, // AVC Intra 50 High 10 {{ 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0a,0x04,0x01,0x02,0x02,0x01,0x32,0x21,0x01 }, 232960, 0, 1}, // AVC Intra 50 1080i60 //etc etc }; static const int mxf_h264_num_codec_uls = sizeof(mxf_h264_codec_uls)/sizeof(mxf_h264_codec_uls[0]); Then use a little for loop in mxf_parse_h264_frame() to find the matching entry. > + if (desc) > + sc->component_depth = desc->comp[0].depth_minus1 + 1; Seems unrelated? In general I didn't check how similar this patch is to the GPL'd version, so I'm going to trust that this doesn't share anything (except the ULs, which come from the standards). /Tomas
signature.asc
Description: This is a digitally signed message part
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel