| Message ID | 20190411230920.6630-3-cus@passwd.hu |
|---|---|
| State | Superseded |
| Headers | show |
fre 2019-04-12 klockan 01:09 +0200 skrev Marton Balint: > This affects the following samples: > > samples/ffmpeg-bugs/roundup/issue1775/av_seek_frame_failure.mxf > samples/ffmpeg-bugs/trac/ticket1957/16ch.mxf > samples/ffmpeg-bugs/trac/ticket5016/r0.mxf > samples/ffmpeg-bugs/trac/ticket5016/r1.mxf > samples/ffmpeg-bugs/trac/ticket5316/hq.MXF > samples/ffmpeg-bugs/trac/ticket5316/hqx.MXF > > Some AVPacket->pos values are changed because for frame wrapped tracks we point > to the KLV offset and not the data. > > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavformat/mxfdec.c | 26 ++++++++++++++++---------- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index 18c038c3f6..236294880e 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -2144,6 +2144,16 @@ static int mxf_add_metadata_stream(MXFContext *mxf, MXFTrack *track) > return 0; > } > > +static MXFWrappingScheme mxf_get_wrapping_by_body_sid(AVFormatContext *s, int body_sid) > +{ > + for (int i = 0; i < s->nb_streams; i++) { > + MXFTrack *track = s->streams[i]->priv_data; > + if (track && track->body_sid == body_sid && track->wrapping != UnknownWrapped) > + return track->wrapping; Maybe print a warning? Especially if, god forbid, the file mixes ClipWrapping and FrameWrapping.. But now I see this function was merely moved further up, so that could be a separate patch.. > @@ -2553,6 +2563,12 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) > } > } > > + for (int i = 0; i < mxf->fc->nb_streams; i++) { > + MXFTrack *track = mxf->fc->streams[i]->priv_data; > + if (track && track->body_sid && track->wrapping == UnknownWrapped) > + track->wrapping = mxf_get_wrapping_by_body_sid(mxf->fc, track->body_sid); > + } Or maybe put a warning here instead. Vendors should fix their dang MXF muxers.. /Tomas
Hi Tomas, I hope you are doing well > On Apr 14, 2019, at 8:58 AM, Tomas Härdin <tjoppen@acc.umu.se> wrote: > > […] > >> @@ -2553,6 +2563,12 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) >> } >> } >> >> + for (int i = 0; i < mxf->fc->nb_streams; i++) { >> + MXFTrack *track = mxf->fc->streams[i]->priv_data; >> + if (track && track->body_sid && track->wrapping == UnknownWrapped) >> + track->wrapping = mxf_get_wrapping_by_body_sid(mxf->fc, track->body_sid); >> + } > > Or maybe put a warning here instead. Vendors should fix their dang MXF > muxers.. Yeah… Since MXF is a container exclusively used in a professional environment, I believe we could be way more strict than other formats. I also feel we don’t have to support older files that were often broken. — Baptiste
On Sun, 14 Apr 2019, Tomas Härdin wrote: > fre 2019-04-12 klockan 01:09 +0200 skrev Marton Balint: >> This affects the following samples: >> >> samples/ffmpeg-bugs/roundup/issue1775/av_seek_frame_failure.mxf >> samples/ffmpeg-bugs/trac/ticket1957/16ch.mxf >> samples/ffmpeg-bugs/trac/ticket5016/r0.mxf >> samples/ffmpeg-bugs/trac/ticket5016/r1.mxf >> samples/ffmpeg-bugs/trac/ticket5316/hq.MXF >> samples/ffmpeg-bugs/trac/ticket5316/hqx.MXF >> >> Some AVPacket->pos values are changed because for frame wrapped tracks we point >> to the KLV offset and not the data. >> >> > Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavformat/mxfdec.c | 26 ++++++++++++++++---------- >> 1 file changed, 16 insertions(+), 10 deletions(-) >> >> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c >> index 18c038c3f6..236294880e 100644 >> --- a/libavformat/mxfdec.c >> +++ b/libavformat/mxfdec.c >> @@ -2144,6 +2144,16 @@ static int mxf_add_metadata_stream(MXFContext *mxf, MXFTrack *track) >> return 0; >> } >> >> +static MXFWrappingScheme mxf_get_wrapping_by_body_sid(AVFormatContext *s, int body_sid) >> +{ >> + for (int i = 0; i < s->nb_streams; i++) { >> + MXFTrack *track = s->streams[i]->priv_data; >> + if (track && track->body_sid == body_sid && track->wrapping != UnknownWrapped) >> + return track->wrapping; > > Maybe print a warning? Especially if, god forbid, the file mixes > ClipWrapping and FrameWrapping.. > > But now I see this function was merely moved further up, so that could > be a separate patch.. > >> @@ -2553,6 +2563,12 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) >> } >> } >> >> + for (int i = 0; i < mxf->fc->nb_streams; i++) { >> + MXFTrack *track = mxf->fc->streams[i]->priv_data; >> + if (track && track->body_sid && track->wrapping == UnknownWrapped) >> + track->wrapping = mxf_get_wrapping_by_body_sid(mxf->fc, track->body_sid); >> + } > > Or maybe put a warning here instead. Vendors should fix their dang MXF > muxers.. We already have a warning (info) if we encounter a stream with unknown wrapping in mxf_structural_metadata. Based on the samples that are affected by this, wrapping is assumed unknown for some GrassValley HQ/HQX files, probably we should fix our own demuxer here and assume frame wrapped. Although we have no idea if clip wrapped HQ/HQX exists... The other one is some Omneon files with uncompressed audio in "AAF AIFF-AIFC Audio Container". This can just as easily be frame wrapped as clip wrapped, no? Thanks, Marton
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index 18c038c3f6..236294880e 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -2144,6 +2144,16 @@ static int mxf_add_metadata_stream(MXFContext *mxf, MXFTrack *track) return 0; } +static MXFWrappingScheme mxf_get_wrapping_by_body_sid(AVFormatContext *s, int body_sid) +{ + for (int i = 0; i < s->nb_streams; i++) { + MXFTrack *track = s->streams[i]->priv_data; + if (track && track->body_sid == body_sid && track->wrapping != UnknownWrapped) + return track->wrapping; + } + return UnknownWrapped; +} + static int mxf_parse_structural_metadata(MXFContext *mxf) { MXFPackage *material_package = NULL; @@ -2553,6 +2563,12 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) } } + for (int i = 0; i < mxf->fc->nb_streams; i++) { + MXFTrack *track = mxf->fc->streams[i]->priv_data; + if (track && track->body_sid && track->wrapping == UnknownWrapped) + track->wrapping = mxf_get_wrapping_by_body_sid(mxf->fc, track->body_sid); + } + ret = 0; fail_and_free: return ret; @@ -2913,16 +2929,6 @@ static int mxf_parse_handle_partition_or_eof(MXFContext *mxf) return mxf->parsing_backward ? mxf_seek_to_previous_partition(mxf) : 1; } -static MXFWrappingScheme mxf_get_wrapping_by_body_sid(AVFormatContext *s, int body_sid) -{ - for (int i = 0; i < s->nb_streams; i++) { - MXFTrack *track = s->streams[i]->priv_data; - if (track && track->body_sid == body_sid && track->wrapping != UnknownWrapped) - return track->wrapping; - } - return UnknownWrapped; -} - /** * Figures out the proper offset and length of the essence container in each partition */
This affects the following samples: samples/ffmpeg-bugs/roundup/issue1775/av_seek_frame_failure.mxf samples/ffmpeg-bugs/trac/ticket1957/16ch.mxf samples/ffmpeg-bugs/trac/ticket5016/r0.mxf samples/ffmpeg-bugs/trac/ticket5016/r1.mxf samples/ffmpeg-bugs/trac/ticket5316/hq.MXF samples/ffmpeg-bugs/trac/ticket5316/hqx.MXF Some AVPacket->pos values are changed because for frame wrapped tracks we point to the KLV offset and not the data. Signed-off-by: Marton Balint <cus@passwd.hu> --- libavformat/mxfdec.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-)