| Message ID | 8E129170-900A-4E57-990F-9FB6D1EF922A@cine.dev |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] mxfenc.c: look for reel_name within streams as well | expand |
On Wed, Sep 6, 2023 at 12:16 PM Bart Styczen <bart.styczen@cine.dev> wrote: > If an MXF file has reel_name stored in a stream, while copying or remuxing > file, some (most) of the stream metadata is lost. > Here’s the file: https://streams.videolan.org/ffmpeg/incoming/original.mxf > After running ffmpeg -i original.mxf -c copy output.mxf, and checking with > ffprobe, > the original file has the following stream metadata: > > file_package_umid: > 0x060A2B340101010501010D431300000064F640280FA57031060E2B347F7F2A80 > > file_package_name: B002C0006_230803_1932_000001 (0) > > track_name : B002C0006_230803_1932_000001 (0)_v1 > > reel_umid : > 0x060A2B340101010501010D431300000064F640280FA67031060E2B347F7F2A80 > > reel_name : B002C0006_230803_1932_000001 (1) > > timecode : 00:06:15:21 > > And the copied file only these: > > file_package_umid: > 0x060A2B340101010501010D001339242252947134203924220052947134203901 > > file_package_name: B002C0006_230803_1932_000001 (0) > > > After applying the patch, copied file correctly shows reel metadata and > the timecode: > > file_package_umid: > 0x060A2B340101010501010D001321CA09529471342721CA090052947134272101 > > file_package_name: B002C0006_230803_1932_000001 (0) > > reel_umid : > 0x060A2B340101010501010D001321CA09529471342721CA090052947134272102 > > reel_name : B002C0006_230803_1932_000001 (1) > > timecode : 00:06:15:21 > > > The track_name is still missing though, so I will continue to investigate > the issue. > > Signed-off-by: Bart Styczen <bart.styczen@cine.dev> > --- > libavformat/mxfenc.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > index d8252ed68f..d3ae83a9fb 100644 > --- a/libavformat/mxfenc.c > +++ b/libavformat/mxfenc.c > @@ -1817,7 +1817,17 @@ static int > mxf_write_header_metadata_sets(AVFormatContext *s) > } > > entry = av_dict_get(s->metadata, "reel_name", NULL, 0); > - if (entry) { > + if (!entry) { > + /* check if any of the streams contain a reel_name */ > + for (i = 0; i < s->nb_streams; i++) { > + st = s->streams[i]; > + if (entry = av_dict_get(st->metadata, "reel_name", NULL, 0)) { > + break; > + } > + } > + } > + > + if(entry) { > Space missing: if (entry) { packages[2].name = entry->value; > packages[2].type = SourcePackage; > packages[2].instance = 2; > -- > 2.37.1 (Apple Git-137.1) > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Signed-off-by: Bart Styczen <bart.styczen@cine.dev> --- libavformat/mxfenc.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index d8252ed68f..d3ae83a9fb 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -1817,7 +1817,17 @@ static int mxf_write_header_metadata_sets(AVFormatContext *s) } entry = av_dict_get(s->metadata, "reel_name", NULL, 0); - if (entry) { + if (!entry) { + /* check if any of the streams contain a reel_name */ + for (i = 0; i < s->nb_streams; i++) { + st = s->streams[i]; + if (entry = av_dict_get(st->metadata, "reel_name", NULL, 0)) { + break; + } + } + } + + if (entry) { packages[2].name = entry->value; packages[2].type = SourcePackage; packages[2].instance = 2; -- 2.37.1 (Apple Git-137.1) On Wed, 6 Sept 2023 at 12:19, Paul B Mahol <onemda@gmail.com> wrote: > On Wed, Sep 6, 2023 at 12:16 PM Bart Styczen <bart.styczen@cine.dev> > wrote: > > > If an MXF file has reel_name stored in a stream, while copying or > remuxing > > file, some (most) of the stream metadata is lost. > > Here’s the file: > https://streams.videolan.org/ffmpeg/incoming/original.mxf > > After running ffmpeg -i original.mxf -c copy output.mxf, and checking > with > > ffprobe, > > the original file has the following stream metadata: > > > file_package_umid: > > 0x060A2B340101010501010D431300000064F640280FA57031060E2B347F7F2A80 > > > file_package_name: B002C0006_230803_1932_000001 (0) > > > track_name : B002C0006_230803_1932_000001 (0)_v1 > > > reel_umid : > > 0x060A2B340101010501010D431300000064F640280FA67031060E2B347F7F2A80 > > > reel_name : B002C0006_230803_1932_000001 (1) > > > timecode : 00:06:15:21 > > > > And the copied file only these: > > > file_package_umid: > > 0x060A2B340101010501010D001339242252947134203924220052947134203901 > > > file_package_name: B002C0006_230803_1932_000001 (0) > > > > > > After applying the patch, copied file correctly shows reel metadata and > > the timecode: > > > file_package_umid: > > 0x060A2B340101010501010D001321CA09529471342721CA090052947134272101 > > > file_package_name: B002C0006_230803_1932_000001 (0) > > > reel_umid : > > 0x060A2B340101010501010D001321CA09529471342721CA090052947134272102 > > > reel_name : B002C0006_230803_1932_000001 (1) > > > timecode : 00:06:15:21 > > > > > > The track_name is still missing though, so I will continue to investigate > > the issue. > > > > Signed-off-by: Bart Styczen <bart.styczen@cine.dev> > > --- > > libavformat/mxfenc.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > > index d8252ed68f..d3ae83a9fb 100644 > > --- a/libavformat/mxfenc.c > > +++ b/libavformat/mxfenc.c > > @@ -1817,7 +1817,17 @@ static int > > mxf_write_header_metadata_sets(AVFormatContext *s) > > } > > > > entry = av_dict_get(s->metadata, "reel_name", NULL, 0); > > - if (entry) { > > + if (!entry) { > > + /* check if any of the streams contain a reel_name */ > > + for (i = 0; i < s->nb_streams; i++) { > > + st = s->streams[i]; > > + if (entry = av_dict_get(st->metadata, "reel_name", NULL, > 0)) { > > + break; > > + } > > + } > > + } > > + > > + if(entry) { > > > > Space missing: if (entry) { > > packages[2].name = entry->value; > > packages[2].type = SourcePackage; > > packages[2].instance = 2; > > -- > > 2.37.1 (Apple Git-137.1) > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
ons 2023-09-06 klockan 12:16 +0200 skrev Bart Styczen: > If an MXF file has reel_name stored in a stream, while copying or > remuxing file, some (most) of the stream metadata is lost. > Here’s the file: > https://streams.videolan.org/ffmpeg/incoming/original.mxf > After running ffmpeg -i original.mxf -c copy output.mxf, and checking > with ffprobe, > the original file has the following stream metadata: > > file_package_umid: > > 0x060A2B340101010501010D431300000064F640280FA57031060E2B347F7F2A80 > > file_package_name: B002C0006_230803_1932_000001 (0) > > track_name : B002C0006_230803_1932_000001 (0)_v1 > > reel_umid : > > 0x060A2B340101010501010D431300000064F640280FA67031060E2B347F7F2A80 > > reel_name : B002C0006_230803_1932_000001 (1) > > timecode : 00:06:15:21 > > And the copied file only these: > > file_package_umid: > > 0x060A2B340101010501010D001339242252947134203924220052947134203901 > > file_package_name: B002C0006_230803_1932_000001 (0) > > > After applying the patch, copied file correctly shows reel metadata > and the timecode: > > file_package_umid: > > 0x060A2B340101010501010D001321CA09529471342721CA090052947134272101 > > file_package_name: B002C0006_230803_1932_000001 (0) > > reel_umid : > > 0x060A2B340101010501010D001321CA09529471342721CA090052947134272102 > > reel_name : B002C0006_230803_1932_000001 (1) > > timecode : 00:06:15:21 > > > The track_name is still missing though, so I will continue to > investigate the issue. Sounds like a mismatch in behavior between mxfdec and mxfenc. Timecode etc going missing suggests a deeper issue to which this patch is likely the wrong fix.. You should report this on trac if you haven't already /Tomas
I will report it right away. Timecode does not really go missing, it is written into the general metadata of the file instead of the video stream metadata. Funny behaviour is that the patch in question, despite only fixing the reel_name, brings back the other remaining metadata into the video stream. I know next to nothing about MXF, this is also my first encounter with FFMPEG, but this smells like a broken linked list with a middle item not being written properly, hence the remaining ones disappear as well. > On 6 Sep 2023, at 20:29, Tomas Härdin <git@haerdin.se> wrote: > > ons 2023-09-06 klockan 12:16 +0200 skrev Bart Styczen: >> If an MXF file has reel_name stored in a stream, while copying or >> remuxing file, some (most) of the stream metadata is lost. >> Here’s the file: >> https://streams.videolan.org/ffmpeg/incoming/original.mxf >> After running ffmpeg -i original.mxf -c copy output.mxf, and checking >> with ffprobe, >> the original file has the following stream metadata: >>> file_package_umid: >>> 0x060A2B340101010501010D431300000064F640280FA57031060E2B347F7F2A80 >>> file_package_name: B002C0006_230803_1932_000001 (0) >>> track_name : B002C0006_230803_1932_000001 (0)_v1 >>> reel_umid : >>> 0x060A2B340101010501010D431300000064F640280FA67031060E2B347F7F2A80 >>> reel_name : B002C0006_230803_1932_000001 (1) >>> timecode : 00:06:15:21 >> >> And the copied file only these: >>> file_package_umid: >>> 0x060A2B340101010501010D001339242252947134203924220052947134203901 >>> file_package_name: B002C0006_230803_1932_000001 (0) >> >> >> After applying the patch, copied file correctly shows reel metadata >> and the timecode: >>> file_package_umid: >>> 0x060A2B340101010501010D001321CA09529471342721CA090052947134272101 >>> file_package_name: B002C0006_230803_1932_000001 (0) >>> reel_umid : >>> 0x060A2B340101010501010D001321CA09529471342721CA090052947134272102 >>> reel_name : B002C0006_230803_1932_000001 (1) >>> timecode : 00:06:15:21 >> >> >> The track_name is still missing though, so I will continue to >> investigate the issue. > > Sounds like a mismatch in behavior between mxfdec and mxfenc. Timecode > etc going missing suggests a deeper issue to which this patch is likely > the wrong fix.. > > You should report this on trac if you haven't already > > /Tomas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index d8252ed68f..d3ae83a9fb 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -1817,7 +1817,17 @@ static int mxf_write_header_metadata_sets(AVFormatContext *s) } entry = av_dict_get(s->metadata, "reel_name", NULL, 0); - if (entry) { + if (!entry) { + /* check if any of the streams contain a reel_name */ + for (i = 0; i < s->nb_streams; i++) { + st = s->streams[i]; + if (entry = av_dict_get(st->metadata, "reel_name", NULL, 0)) { + break; + } + } + } + + if(entry) { packages[2].name = entry->value; packages[2].type = SourcePackage; packages[2].instance = 2;