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 [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
