On Tue, 2014-11-25 at 15:14 -0800, Mark Reid wrote: > --- > libavformat/mxf.c | 1 + > libavformat/mxf.h | 1 + > libavformat/mxfdec.c | 148 > ++++++++++++++++++++++++++++++++++++++++--------- > tests/ref/lavf/mxf | 6 +- > tests/ref/lavf/mxf_d10 | 2 +- > 5 files changed, 127 insertions(+), 31 deletions(-) > > diff --git a/libavformat/mxf.c b/libavformat/mxf.c > index 4dc54d7..14d143e 100644 > --- a/libavformat/mxf.c > +++ b/libavformat/mxf.c > @@ -94,6 +94,7 @@ static const struct { > {AV_PIX_FMT_RGB565BE,{'R', 5, 'G', 6, 'B', 5 > }}, > {AV_PIX_FMT_RGBA, {'R', 8, 'G', 8, 'B', 8, 'A', 8 > }}, > {AV_PIX_FMT_PAL8, {'P', 8 > }}, > + {AV_PIX_FMT_GRAY8, {'A', 8 > }},
There's no pixel format for pure alpha? > }; > > static const int num_pixel_layouts = FF_ARRAY_ELEMS(ff_mxf_pixel_layouts); > diff --git a/libavformat/mxf.h b/libavformat/mxf.h > index 5b95efa..63b497a 100644 > --- a/libavformat/mxf.h > +++ b/libavformat/mxf.h > @@ -35,6 +35,7 @@ enum MXFMetadataSetType { > TimecodeComponent, > PulldownComponent, > Sequence, > + EssenceGroup, Add this to the end of the enum? That way mxfenc output doesn't change. > MultipleDescriptor, > Descriptor, > Track, > [...] > > +static int mxf_read_essence_group(void *arg, AVIOContext *pb, int tag, int > size, UID uid, int64_t klv_offset) > +{ > + MXFEssenceGroup *essence_group = arg; > + switch (tag) { > + case 0x0202: > + essence_group->duration = avio_rb64(pb); > + break; > + case 0x0501: > + essence_group->structural_components_count = avio_rb32(pb); > + essence_group->structural_components_refs = > av_calloc(essence_group->structural_components_count, sizeof(UID)); > + if (!essence_group->structural_components_refs) > + return AVERROR(ENOMEM); > + avio_skip(pb, 4); /* useless size of objects, always 16 according to > specs */ > + avio_read(pb, (uint8_t *)essence_group->structural_components_refs, > essence_group->structural_components_count * sizeof(UID)); Is there any risk of this multiplication overflowing? I suspect av_calloc() will fail if it does, just making sure. Also not critical if it actually does since avio_read() just ends up reading less. > +static MXFStructuralComponent* mxf_resolve_essence_group_choice(MXFContext > *mxf, MXFEssenceGroup *essence_group) > +{ > + MXFStructuralComponent *component = NULL; > + MXFPackage *package = NULL; > + MXFDescriptor *descriptor = NULL; > + int i; > + > + if (!essence_group || !essence_group->structural_components_count) > + return NULL; > + > + /* essence groups contains multiple representations of the same media, > + this return the first components with a valid Descriptor typically > index 0 */ > + for (i =0; i < essence_group->structural_components_count; i++){ > + component = mxf_resolve_strong_ref(mxf, > &essence_group->structural_components_refs[i], SourceClip); > + if (!component) > + continue; > + > + if (!(package = mxf_resolve_source_package(mxf, > component->source_package_uid))) > + continue; > + > + descriptor = mxf_resolve_strong_ref(mxf, &package->descriptor_ref, > Descriptor); > + if (descriptor){ > + /* HACK: force the duration of the component to match the > duration of the descriptor */ > + if (descriptor->duration != AV_NOPTS_VALUE) > + component->duration = descriptor->duration; Not a huge fan of this, but probably doesn't hurt since it's checking for AV_NOPTS_VALUE. > +static int mxf_metadataset_init(MXFMetadataSet *ctx, enum MXFMetadataSetType > type) > +{ > + switch (type){ > + case MultipleDescriptor: > + case Descriptor: > + ((MXFDescriptor*)ctx)->pix_fmt = AV_PIX_FMT_NONE; > + ((MXFDescriptor*)ctx)->duration = AV_NOPTS_VALUE; > + break; > + default: > + break; > + } > + return 0; > +} > + Good idea :) > diff --git a/tests/ref/lavf/mxf b/tests/ref/lavf/mxf > index 236661c..d237560 100644 > --- a/tests/ref/lavf/mxf > +++ b/tests/ref/lavf/mxf Again, probably not needed if you stick EssenceGroup at the end of the enum. I like using mxf_resolve_source_package() to reduce the size of mxf_parse_physical_source_package() and mxf_parse_structural_metadata(). Memory handling looks correct too. Did you double-check with valgrind? Looks pretty good overall. /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