| Message ID | 20220828153123.10813-1-pal@sandflow.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v1] avformat/mxf: set stream frame rates for ST 422 essence containers | expand |
sön 2022-08-28 klockan 08:31 -0700 skrev pal@sandflow.com: > From: Pierre-Anthony Lemieux <pal@palemieux.com> > > The MXF demuxer does not currently set AVStream::avg_frame_rate and > ::r_frame_rate > when J2K essence is wrapped according to SMPTE ST 422. > > --- > libavformat/mxfdec.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index feebff67aa..043e2e06ec 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -2140,6 +2140,13 @@ finish_decoding_index: > return ret; > } > > +static int mxf_is_st_422(const UID *essence_container_ul) { > + static const uint8_t st_422_essence_container_ul[] = { > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c > }; > + > + return essence_container_ul && > mxf_match_uid(*essence_container_ul, st_422_essence_container_ul, > + > sizeof(st_422_essence_container_ul)); > +} Seems unnecessary to have a function for this > + > static int mxf_is_intra_only(MXFDescriptor *descriptor) > { > return mxf_get_codec_ul(mxf_intra_only_essence_container_uls, > @@ -2892,6 +2899,24 @@ static int > mxf_parse_structural_metadata(MXFContext *mxf) > av_log(mxf->fc, AV_LOG_INFO, "Unknown frame > layout type: %d\n", descriptor->frame_layout); > } > > + if (mxf_is_st_422(essence_container_ul)) { > + switch ((*essence_container_ul)[14]) { > + case 2: /* Cn: Clip- wrapped Picture Element */ > + case 3: /* I1: Interlaced Frame, 1 field/KLV */ > + case 4: /* I2: Interlaced Frame, 2 fields/KLV */ > + case 6: /* P1: Frame- wrapped Picture Element */ > + st->avg_frame_rate = source_track->edit_rate; > + st->r_frame_rate = st->avg_frame_rate; > + break; > + case 5: /* F1: Field-wrapped Picture Element */ > + st->avg_frame_rate = av_mul_q(av_make_q(2, 1), > source_track->edit_rate); > + st->r_frame_rate = st->avg_frame_rate; > + break; > + default: > + break; > + } > + } Looks OK, but we should have a test for this as well I think At some point we'll probably need to implement proper timecode support.. /Tomas
On Sat, Sep 3, 2022 at 11:26 AM Tomas Härdin <tjoppen@acc.umu.se> wrote: > > sön 2022-08-28 klockan 08:31 -0700 skrev pal@sandflow.com: > > From: Pierre-Anthony Lemieux <pal@palemieux.com> > > > > The MXF demuxer does not currently set AVStream::avg_frame_rate and > > ::r_frame_rate > > when J2K essence is wrapped according to SMPTE ST 422. > > > > --- > > libavformat/mxfdec.c | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > > index feebff67aa..043e2e06ec 100644 > > --- a/libavformat/mxfdec.c > > +++ b/libavformat/mxfdec.c > > @@ -2140,6 +2140,13 @@ finish_decoding_index: > > return ret; > > } > > > > +static int mxf_is_st_422(const UID *essence_container_ul) { > > + static const uint8_t st_422_essence_container_ul[] = { > > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c > > }; > > + > > + return essence_container_ul && > > mxf_match_uid(*essence_container_ul, st_422_essence_container_ul, > > + > > sizeof(st_422_essence_container_ul)); > > +} > > Seems unnecessary to have a function for this Ok. I was trying to avoid a long if() line but I am happy to move the logic to mxf_parse_structural_metadata(). Would you like me to update the patch? > > > + > > static int mxf_is_intra_only(MXFDescriptor *descriptor) > > { > > return mxf_get_codec_ul(mxf_intra_only_essence_container_uls, > > @@ -2892,6 +2899,24 @@ static int > > mxf_parse_structural_metadata(MXFContext *mxf) > > av_log(mxf->fc, AV_LOG_INFO, "Unknown frame > > layout type: %d\n", descriptor->frame_layout); > > } > > > > + if (mxf_is_st_422(essence_container_ul)) { > > + switch ((*essence_container_ul)[14]) { > > + case 2: /* Cn: Clip- wrapped Picture Element */ > > + case 3: /* I1: Interlaced Frame, 1 field/KLV */ > > + case 4: /* I2: Interlaced Frame, 2 fields/KLV */ > > + case 6: /* P1: Frame- wrapped Picture Element */ > > + st->avg_frame_rate = source_track->edit_rate; > > + st->r_frame_rate = st->avg_frame_rate; > > + break; > > + case 5: /* F1: Field-wrapped Picture Element */ > > + st->avg_frame_rate = av_mul_q(av_make_q(2, 1), > > source_track->edit_rate); > > + st->r_frame_rate = st->avg_frame_rate; > > + break; > > + default: > > + break; > > + } > > + } > > Looks OK, but we should have a test for this as well I think Ok. imf/countdown/countdown-small.mxf contained ST 422-wrapped essence and ffprobe reports `24 fps`. Should this patchset include the test? > > At some point we'll probably need to implement proper timecode > support.. +1 Happy to collaborate. > > /Tomas > > _______________________________________________ > 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".
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index feebff67aa..043e2e06ec 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -2140,6 +2140,13 @@ finish_decoding_index: return ret; } +static int mxf_is_st_422(const UID *essence_container_ul) { + static const uint8_t st_422_essence_container_ul[] = { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c }; + + return essence_container_ul && mxf_match_uid(*essence_container_ul, st_422_essence_container_ul, + sizeof(st_422_essence_container_ul)); +} + static int mxf_is_intra_only(MXFDescriptor *descriptor) { return mxf_get_codec_ul(mxf_intra_only_essence_container_uls, @@ -2892,6 +2899,24 @@ static int mxf_parse_structural_metadata(MXFContext *mxf) av_log(mxf->fc, AV_LOG_INFO, "Unknown frame layout type: %d\n", descriptor->frame_layout); } + if (mxf_is_st_422(essence_container_ul)) { + switch ((*essence_container_ul)[14]) { + case 2: /* Cn: Clip- wrapped Picture Element */ + case 3: /* I1: Interlaced Frame, 1 field/KLV */ + case 4: /* I2: Interlaced Frame, 2 fields/KLV */ + case 6: /* P1: Frame- wrapped Picture Element */ + st->avg_frame_rate = source_track->edit_rate; + st->r_frame_rate = st->avg_frame_rate; + break; + case 5: /* F1: Field-wrapped Picture Element */ + st->avg_frame_rate = av_mul_q(av_make_q(2, 1), source_track->edit_rate); + st->r_frame_rate = st->avg_frame_rate; + break; + default: + break; + } + } + if (st->codecpar->codec_id == AV_CODEC_ID_PRORES) { switch (descriptor->essence_codec_ul[14]) { case 1: st->codecpar->codec_tag = MKTAG('a','p','c','o'); break;
From: Pierre-Anthony Lemieux <pal@palemieux.com> The MXF demuxer does not currently set AVStream::avg_frame_rate and ::r_frame_rate when J2K essence is wrapped according to SMPTE ST 422. --- libavformat/mxfdec.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)