Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/mxfenc: write reel_name if metadata key is present
On Fri, Dec 08, 2017 at 12:19:21AM +0100, Tomas Härdin wrote: > On Thu, 2017-12-07 at 14:45 -0800, Mark Reid wrote: > > On Dec 7, 2017 7:45 AM, "Tomas Härdin" wrote: > > > > On 2017-12-05 05:46, Mark Reid wrote: > > > > > --- > > > libavformat/mxf.h| 1 + > > > libavformat/mxfenc.c | 42 +++-- > > > - > > > 2 files changed, 36 insertions(+), 7 deletions(-) > > > > > > > > > @@ -1476,6 +1495,15 @@ static int > > > mxf_write_header_metadata_sets(AVFormatContext > > > *s) > > > } > > > } > > > +entry = av_dict_get(s->metadata, "reel_name", NULL, 0); > > > +if (entry) { > > > +packages[2].name = entry->value; > > > +packages[2].type = SourcePackage; > > > +packages[2].instance = 2; > > > +packages[1].ref = &packages[2]; > > > +package_count = 3; > > > +} > > > > > > > I guess we can have it check track metadata later if we feel like it. > > Did a > > patch moving reel_name into AVFormatContext make it into mxfdec yet? > > mxfenc's output surviving a roundtrip through mxfdec + mxfenc might > > be > > desirable: > > > > ffmpeg -i somefile_with_reel_name.mkv output.mxf > > ffmpeg -i output.mxf -vcodec copy -acodec copy remuxed.mxf > > > > Ideally remuxed.mxf is identical to output.mxf > > > > /Tomas > > > > > > Yes I agree that such behaviour is desirable. > > I haven't taken a look at mxfdec yet. Looking in steams for reel_name > > might > > be necessary, as I believe that's where mov,MP4 store it. But I was > > trying > > to keep it simple at first and addressed this specific issue a future > > patch. > > Fair enough. I guess we can commit this patch series then. Michael? will apply thanks [...] -- 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 signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/mxfenc: write reel_name if metadata key is present
On Thu, 2017-12-07 at 14:45 -0800, Mark Reid wrote: > On Dec 7, 2017 7:45 AM, "Tomas Härdin" wrote: > > On 2017-12-05 05:46, Mark Reid wrote: > > > --- > > libavformat/mxf.h| 1 + > > libavformat/mxfenc.c | 42 +++-- > > - > > 2 files changed, 36 insertions(+), 7 deletions(-) > > > > > > @@ -1476,6 +1495,15 @@ static int > > mxf_write_header_metadata_sets(AVFormatContext > > *s) > > } > > } > > +entry = av_dict_get(s->metadata, "reel_name", NULL, 0); > > +if (entry) { > > +packages[2].name = entry->value; > > +packages[2].type = SourcePackage; > > +packages[2].instance = 2; > > +packages[1].ref = &packages[2]; > > +package_count = 3; > > +} > > > > I guess we can have it check track metadata later if we feel like it. > Did a > patch moving reel_name into AVFormatContext make it into mxfdec yet? > mxfenc's output surviving a roundtrip through mxfdec + mxfenc might > be > desirable: > > ffmpeg -i somefile_with_reel_name.mkv output.mxf > ffmpeg -i output.mxf -vcodec copy -acodec copy remuxed.mxf > > Ideally remuxed.mxf is identical to output.mxf > > /Tomas > > > Yes I agree that such behaviour is desirable. > I haven't taken a look at mxfdec yet. Looking in steams for reel_name > might > be necessary, as I believe that's where mov,MP4 store it. But I was > trying > to keep it simple at first and addressed this specific issue a future > patch. Fair enough. I guess we can commit this patch series then. Michael? /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
Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/mxfenc: write reel_name if metadata key is present
On Dec 7, 2017 7:45 AM, "Tomas Härdin" wrote: On 2017-12-05 05:46, Mark Reid wrote: > --- > libavformat/mxf.h| 1 + > libavformat/mxfenc.c | 42 +++--- > 2 files changed, 36 insertions(+), 7 deletions(-) > > > @@ -1476,6 +1495,15 @@ static int > mxf_write_header_metadata_sets(AVFormatContext > *s) > } > } > +entry = av_dict_get(s->metadata, "reel_name", NULL, 0); > +if (entry) { > +packages[2].name = entry->value; > +packages[2].type = SourcePackage; > +packages[2].instance = 2; > +packages[1].ref = &packages[2]; > +package_count = 3; > +} > I guess we can have it check track metadata later if we feel like it. Did a patch moving reel_name into AVFormatContext make it into mxfdec yet? mxfenc's output surviving a roundtrip through mxfdec + mxfenc might be desirable: ffmpeg -i somefile_with_reel_name.mkv output.mxf ffmpeg -i output.mxf -vcodec copy -acodec copy remuxed.mxf Ideally remuxed.mxf is identical to output.mxf /Tomas Yes I agree that such behaviour is desirable. I haven't taken a look at mxfdec yet. Looking in steams for reel_name might be necessary, as I believe that's where mov,MP4 store it. But I was trying to keep it simple at first and addressed this specific issue a future patch. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/mxfenc: write reel_name if metadata key is present
On 2017-12-05 05:46, Mark Reid wrote: --- libavformat/mxf.h| 1 + libavformat/mxfenc.c | 42 +++--- 2 files changed, 36 insertions(+), 7 deletions(-) @@ -1476,6 +1495,15 @@ static int mxf_write_header_metadata_sets(AVFormatContext *s) } } +entry = av_dict_get(s->metadata, "reel_name", NULL, 0); +if (entry) { +packages[2].name = entry->value; +packages[2].type = SourcePackage; +packages[2].instance = 2; +packages[1].ref = &packages[2]; +package_count = 3; +} I guess we can have it check track metadata later if we feel like it. Did a patch moving reel_name into AVFormatContext make it into mxfdec yet? mxfenc's output surviving a roundtrip through mxfdec + mxfenc might be desirable: ffmpeg -i somefile_with_reel_name.mkv output.mxf ffmpeg -i output.mxf -vcodec copy -acodec copy remuxed.mxf Ideally remuxed.mxf is identical to output.mxf /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v3 2/3] avformat/mxfenc: write reel_name if metadata key is present
--- libavformat/mxf.h| 1 + libavformat/mxfenc.c | 42 +++--- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/libavformat/mxf.h b/libavformat/mxf.h index 2d5b44943b..ffcc429a8b 100644 --- a/libavformat/mxf.h +++ b/libavformat/mxf.h @@ -47,6 +47,7 @@ enum MXFMetadataSetType { EssenceContainerData, EssenceGroup, TaggedValue, +TapeDescriptor, }; enum MXFFrameLayout { diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index c9694f2a4e..3bb70326fe 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -105,6 +105,7 @@ typedef struct MXFPackage { char *name; enum MXFMetadataSetType type; int instance; +struct MXFPackage *ref; } MXFPackage; enum ULIndex { @@ -991,20 +992,33 @@ static void mxf_write_structural_component(AVFormatContext *s, AVStream *st, MXF // write source package uid, end of the reference mxf_write_local_tag(pb, 32, 0x1101); -if (package->type == SourcePackage) { +if (!package->ref) { for (i = 0; i < 4; i++) avio_wb64(pb, 0); } else -mxf_write_umid(s, 1); +mxf_write_umid(s, package->ref->instance); // write source track id mxf_write_local_tag(pb, 4, 0x1102); -if (package->type == SourcePackage) +if (package->type == SourcePackage && !package->ref) avio_wb32(pb, 0); else avio_wb32(pb, st->index+2); } +static void mxf_write_tape_descriptor(AVFormatContext *s) +{ +AVIOContext *pb = s->pb; + +mxf_write_metadata_key(pb, 0x012e00); +PRINT_KEY(s, "tape descriptor key", pb->buf_ptr - 16); +klv_encode_ber_length(pb, 20); +mxf_write_local_tag(pb, 16, 0x3C0A); +mxf_write_uuid(pb, TapeDescriptor, 0); +PRINT_KEY(s, "tape_desc uid", pb->buf_ptr - 16); +} + + static void mxf_write_multi_descriptor(AVFormatContext *s) { MXFContext *mxf = s->priv_data; @@ -1388,13 +1402,17 @@ static void mxf_write_package(AVFormatContext *s, MXFPackage *package) } // write multiple descriptor reference -if (package->type == SourcePackage) { +if (package->type == SourcePackage && package->instance == 1) { mxf_write_local_tag(pb, 16, 0x4701); if (s->nb_streams > 1) { mxf_write_uuid(pb, MultipleDescriptor, 0); mxf_write_multi_descriptor(s); } else mxf_write_uuid(pb, SubDescriptor, 0); +} else if (package->type == SourcePackage && package->instance == 2) { +mxf_write_local_tag(pb, 16, 0x4701); +mxf_write_uuid(pb, TapeDescriptor, 0); +mxf_write_tape_descriptor(s); } /* @@ -1418,7 +1436,7 @@ static void mxf_write_package(AVFormatContext *s, MXFPackage *package) mxf_write_structural_component(s, st, package); mxf->track_instance_count++; -if (package->type == SourcePackage) { +if (package->type == SourcePackage && package->instance == 1) { MXFStreamContext *sc = st->priv_data; mxf_essence_container_uls[sc->index].write_desc(s, st); } @@ -1453,12 +1471,13 @@ static int mxf_write_header_metadata_sets(AVFormatContext *s) AVDictionaryEntry *entry = NULL; AVStream *st = NULL; int i; - -MXFPackage packages[2] = {{0}}; +MXFPackage packages[3] = {{0}}; int package_count = 2; packages[0].type = MaterialPackage; packages[1].type = SourcePackage; packages[1].instance = 1; +packages[0].ref = &packages[1]; + if (entry = av_dict_get(s->metadata, "material_package_name", NULL, 0)) packages[0].name = entry->value; @@ -1476,6 +1495,15 @@ static int mxf_write_header_metadata_sets(AVFormatContext *s) } } +entry = av_dict_get(s->metadata, "reel_name", NULL, 0); +if (entry) { +packages[2].name = entry->value; +packages[2].type = SourcePackage; +packages[2].instance = 2; +packages[1].ref = &packages[2]; +package_count = 3; +} + mxf_write_preface(s); mxf_write_identification(s); mxf_write_content_storage(s, packages, package_count); -- 2.13.6 (Apple Git-96) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel