| Message ID | AM7PR03MB6660DFAD9070239D5A897B9A8F929@AM7PR03MB6660.eurprd03.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,01/17] avformat/mxfenc: Auto-insert h264_mp4toannexb BSF if needed | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt: > These tests exhibit two bugs: Instead of using the in-band extradata > the demuxer makes up some extradata designed for AVC intra tracks > that lack in-band extradata; these files are nevertheless decodable > because of the in-band extradata. Furthermore, the frame reordering > is lost. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > tests/fate/mxf.mak | 22 ++++++++++- > tests/ref/fate/mxf-remux-h264 | 37 ++++++++++++++++++ > tests/ref/fate/mxf-remux-xavc | 71 > +++++++++++++++++++++++++++++++++++ > 3 files changed, 128 insertions(+), 2 deletions(-) > create mode 100644 tests/ref/fate/mxf-remux-h264 > create mode 100644 tests/ref/fate/mxf-remux-xavc > > diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak > index f96f4a429b..58a697cd86 100644 > --- a/tests/fate/mxf.mak > +++ b/tests/fate/mxf.mak > @@ -42,6 +42,21 @@ FATE_MXF_REMUX_PROBE-$(call ALLYES, PRORES_DECODER > MXF_MUXER) \ > += fate-mxf-remux-applehdr10 > fate-mxf-remux-applehdr10: CMD = transcode mxf > $(TARGET_SAMPLES)/mxf/Meridian-Apple_ProResProxy-HDR10.mxf mxf "-map > 0 -c copy" "-c copy -t 0.3" "" "-show_entries > format_tags:stream_side_data_list:stream=index,codec_name,codec_tag:s > tream_tags" > > +# Tests muxing H.264, in particular automatic insertion of > h264_mp4toannexb. > +# FIXME: The timestamps of the demuxed file are not properly > reordered. > +# Furthermore the extradata is wrong: It is one of the AVC intra > SPS/PPS; > +# decoding only works due to in-band extradata. Is this a problem with the samples or the code? Or both? /Tomas
Tomas Härdin: > tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt: >> These tests exhibit two bugs: Instead of using the in-band extradata >> the demuxer makes up some extradata designed for AVC intra tracks >> that lack in-band extradata; these files are nevertheless decodable >> because of the in-band extradata. Furthermore, the frame reordering >> is lost. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> tests/fate/mxf.mak | 22 ++++++++++- >> tests/ref/fate/mxf-remux-h264 | 37 ++++++++++++++++++ >> tests/ref/fate/mxf-remux-xavc | 71 >> +++++++++++++++++++++++++++++++++++ >> 3 files changed, 128 insertions(+), 2 deletions(-) >> create mode 100644 tests/ref/fate/mxf-remux-h264 >> create mode 100644 tests/ref/fate/mxf-remux-xavc >> >> diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak >> index f96f4a429b..58a697cd86 100644 >> --- a/tests/fate/mxf.mak >> +++ b/tests/fate/mxf.mak >> @@ -42,6 +42,21 @@ FATE_MXF_REMUX_PROBE-$(call ALLYES, PRORES_DECODER >> MXF_MUXER) \ >> += fate-mxf-remux-applehdr10 >> fate-mxf-remux-applehdr10: CMD = transcode mxf >> $(TARGET_SAMPLES)/mxf/Meridian-Apple_ProResProxy-HDR10.mxf mxf "-map >> 0 -c copy" "-c copy -t 0.3" "" "-show_entries >> format_tags:stream_side_data_list:stream=index,codec_name,codec_tag:s >> tream_tags" >> >> +# Tests muxing H.264, in particular automatic insertion of >> h264_mp4toannexb. >> +# FIXME: The timestamps of the demuxed file are not properly >> reordered. >> +# Furthermore the extradata is wrong: It is one of the AVC intra >> SPS/PPS; >> +# decoding only works due to in-band extradata. > > Is this a problem with the samples or the code? Or both? > The extradata issue is due to a bug in the demuxer: It always adds extradata for H.264 instead of letting extract_extradata_bsf extract it from the bitstream. See these lines here: https://github.com/FFmpeg/FFmpeg/blob/447cf537746cd9969674ebbd60411b6093603c59/libavformat/mxfdec.c#L2711-L2718 The solution for this is of course to detect whether we are dealing with AVC intra data and only adding the extradata in this case. What is the right way to check for this? Is it the presence of the H.264 ul in mxf_intra_only_picture_essence_coding_uls? (Another way would be to check whether the first packet has extradata, but this is probably more involved (we would basically have to reimplement/reuse a part of avformat_find_stream_info() (the part that deals with extract_extradata) ourselves).) I don't know whether the timestamps are due to a bug in the muxer or the demuxer, but the timestamps the muxer receives are properly reordered. - Andreas
ons 2021-11-10 klockan 14:52 +0100 skrev Andreas Rheinhardt: > Tomas Härdin: > > tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt: > > > These tests exhibit two bugs: Instead of using the in-band > > > extradata > > > the demuxer makes up some extradata designed for AVC intra tracks > > > that lack in-band extradata; these files are nevertheless > > > decodable > > > because of the in-band extradata. Furthermore, the frame > > > reordering > > > is lost. > > > > > > Signed-off-by: Andreas Rheinhardt > > > <andreas.rheinhardt@outlook.com> > > > --- > > > tests/fate/mxf.mak | 22 ++++++++++- > > > tests/ref/fate/mxf-remux-h264 | 37 ++++++++++++++++++ > > > tests/ref/fate/mxf-remux-xavc | 71 > > > +++++++++++++++++++++++++++++++++++ > > > 3 files changed, 128 insertions(+), 2 deletions(-) > > > create mode 100644 tests/ref/fate/mxf-remux-h264 > > > create mode 100644 tests/ref/fate/mxf-remux-xavc > > > > > > diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak > > > index f96f4a429b..58a697cd86 100644 > > > --- a/tests/fate/mxf.mak > > > +++ b/tests/fate/mxf.mak > > > @@ -42,6 +42,21 @@ FATE_MXF_REMUX_PROBE-$(call ALLYES, > > > PRORES_DECODER > > > MXF_MUXER) \ > > > += fate-mxf-remux-applehdr10 > > > fate-mxf-remux-applehdr10: CMD = transcode mxf > > > $(TARGET_SAMPLES)/mxf/Meridian-Apple_ProResProxy-HDR10.mxf mxf "- > > > map > > > 0 -c copy" "-c copy -t 0.3" "" "-show_entries > > > format_tags:stream_side_data_list:stream=index,codec_name,codec_t > > > ag:s > > > tream_tags" > > > > > > +# Tests muxing H.264, in particular automatic insertion of > > > h264_mp4toannexb. > > > +# FIXME: The timestamps of the demuxed file are not properly > > > reordered. > > > +# Furthermore the extradata is wrong: It is one of the AVC intra > > > SPS/PPS; > > > +# decoding only works due to in-band extradata. > > > > Is this a problem with the samples or the code? Or both? > > > > The extradata issue is due to a bug in the demuxer: It always adds > extradata for H.264 instead of letting extract_extradata_bsf extract > it > from the bitstream. See these lines here: > https://github.com/FFmpeg/FFmpeg/blob/447cf537746cd9969674ebbd60411b6093603c59/libavformat/mxfdec.c#L2711-L2718 > The solution for this is of course to detect whether we are dealing > with > AVC intra data and only adding the extradata in this case. What is > the > right way to check for this? Is it the presence of the H.264 ul in > mxf_intra_only_picture_essence_coding_uls? (Another way would be to > check whether the first packet has extradata, but this is probably > more > involved (we would basically have to reimplement/reuse a part of > avformat_find_stream_info() (the part that deals with > extract_extradata) > ourselves).) > I don't know whether the timestamps are due to a bug in the muxer or > the > demuxer, but the timestamps the muxer receives are properly > reordered. Answers to questions like these are in the official SMPTE mapping documents. We should not invent our own hacks. Typically a UL is used to signal this, yes. For AVC-Intra, PPS and SPS seem to be implicit, hence ff_generate_avci_extradata(). For other AVC/H.264 mappings it might be that there's a metadata KLV that stores the extradata, or extradata are to be parsed from the essence. Again, the mapping document should specify this. Do *not* add hacks that try to autodetect this stuff. That just encourages even more hacks and deviations from the standards. Hacks beget hacks. At some point we might have to go through mxfdec.c and limit already existing hacks to only apply to files written by specific MXF encoders. /Tomas
diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak index f96f4a429b..58a697cd86 100644 --- a/tests/fate/mxf.mak +++ b/tests/fate/mxf.mak @@ -42,6 +42,21 @@ FATE_MXF_REMUX_PROBE-$(call ALLYES, PRORES_DECODER MXF_MUXER) \ += fate-mxf-remux-applehdr10 fate-mxf-remux-applehdr10: CMD = transcode mxf $(TARGET_SAMPLES)/mxf/Meridian-Apple_ProResProxy-HDR10.mxf mxf "-map 0 -c copy" "-c copy -t 0.3" "" "-show_entries format_tags:stream_side_data_list:stream=index,codec_name,codec_tag:stream_tags" +# Tests muxing H.264, in particular automatic insertion of h264_mp4toannexb. +# FIXME: The timestamps of the demuxed file are not properly reordered. +# Furthermore the extradata is wrong: It is one of the AVC intra SPS/PPS; +# decoding only works due to in-band extradata. +FATE_MXF_REMUX-$(call ALLYES, MOV_DEMUXER H264_MP4TOANNEXB_BSF \ + MXF_MUXER H264_PARSER H264_DECODER \ + RAWVIDEO_ENCODER) += fate-mxf-remux-h264 +fate-mxf-remux-h264: CMD = transcode mov $(TARGET_SAMPLES)/mov/spherical.mov mxf "-c copy" "-map 0 -map 0 -c:v:1 copy -frames:v 12" + +# Tests remuxing H.264 and data streams. +# The same FIXME as for mxf-remux-h264 applies. +FATE_MXF_REMUX-$(call ALLYES, MXF_MUXER H264_PARSER H264_DECODER) \ + += fate-mxf-remux-xavc +fate-mxf-remux-xavc: CMD = transcode mxf $(TARGET_SAMPLES)/h264/SonyXAVC_LongGOP_green_pixelation_early_Frames.MXF mxf "-map 0 -c copy" "-map 0 -c copy" + FATE_MXF_REEL_NAME-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) += fate-mxf-reel_name fate-mxf-reel_name: $(SAMPLES)/mxf/Sony-00001.mxf fate-mxf-reel_name: CMD = md5 -y -i $(TARGET_SAMPLES)/mxf/Sony-00001.mxf -c copy -timecode 00:00:00:00 -metadata "reel_name=test_reel" -fflags +bitexact -f mxf @@ -64,10 +79,13 @@ FATE_MXF-$(CONFIG_MXF_DEMUXER) += $(FATE_MXF) FATE_MXF_FFMPEG_FFPROBE-$(call ALLYES, FILE_PROTOCOL MXF_DEMUXER \ PIPE_PROTOCOL FRAMECRC_MUXER) \ += $(FATE_MXF_REMUX_PROBE-yes) +FATE_MXF_FFMPEG-$(call ALLYES, FILE_PROTOCOL MXF_DEMUXER \ + PIPE_PROTOCOL FRAMECRC_MUXER) \ + += $(FATE_MXF_REMUX-yes) -FATE_SAMPLES_AVCONV += $(FATE_MXF-yes) $(FATE_MXF_REEL_NAME-yes) +FATE_SAMPLES_AVCONV += $(FATE_MXF-yes) $(FATE_MXF_REEL_NAME-yes) $(FATE_MXF_FFMPEG-yes) FATE_SAMPLES_AVCONV += $(FATE_MXF_USER_COMMENTS-yes) $(FATE_MXF_OPATOM_USER_COMMENTS-yes) FATE_SAMPLES_FFMPEG_FFPROBE += $(FATE_MXF_FFMPEG_FFPROBE-yes) FATE_SAMPLES_FFPROBE += $(FATE_MXF_PROBE-yes) -fate-mxf: $(FATE_MXF-yes) $(FATE_MXF_PROBE-yes) $(FATE_MXF_REEL_NAME-yes) $(FATE_MXF_USER_COMMENTS-yes) $(FATE_MXF_FFMPEG_FFPROBE-yes) $(FATE_MXF_OPATOM_USER_COMMENTS-yes) +fate-mxf: $(FATE_MXF-yes) $(FATE_MXF_PROBE-yes) $(FATE_MXF_REEL_NAME-yes) $(FATE_MXF_USER_COMMENTS-yes) $(FATE_MXF_FFMPEG-yes) $(FATE_MXF_FFMPEG_FFPROBE-yes) $(FATE_MXF_OPATOM_USER_COMMENTS-yes) diff --git a/tests/ref/fate/mxf-remux-h264 b/tests/ref/fate/mxf-remux-h264 new file mode 100644 index 0000000000..49ce4324c4 --- /dev/null +++ b/tests/ref/fate/mxf-remux-h264 @@ -0,0 +1,37 @@ +a06741f1a308fa0bd755b206d93639d2 *tests/data/fate/mxf-remux-h264.mxf +201273 tests/data/fate/mxf-remux-h264.mxf +#extradata 1: 81, 0x98411475 +#tb 0: 1/25 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 1920x1080 +#sar 0: 1/1 +#tb 1: 1/25 +#media_type 1: video +#codec_id 1: h264 +#dimensions 1: 1920x1080 +#sar 1: 1/1 +0, 0, 0, 1, 6220800, 0x6a771857 +1, 0, 0, 1, 69157, 0xd6ff5c4d +1, 1, 1, 1, 1103, 0xb8e00551, F=0x0 +1, 2, 2, 1, 141, 0x49232f95, F=0x0 +0, 3, 3, 1, 6220800, 0x92aa1963 +1, 3, 3, 1, 131, 0xe01928c0, F=0x0 +0, 4, 4, 1, 6220800, 0x906014d8 +1, 4, 4, 1, 69, 0xdf5506bc, F=0x0 +0, 5, 5, 1, 6220800, 0xa67a179e +1, 5, 5, 1, 424, 0x408cb97b, F=0x0 +0, 6, 6, 1, 6220800, 0xcf7e1148 +1, 6, 6, 1, 83, 0x5aa5118a, F=0x0 +0, 7, 7, 1, 6220800, 0xd2b623ab +1, 7, 7, 1, 70, 0x39ec0a38, F=0x0 +0, 8, 8, 1, 6220800, 0x97f51b81 +1, 8, 8, 1, 71, 0x55290a92, F=0x0 +0, 9, 9, 1, 6220800, 0x53e01e46 +1, 9, 9, 1, 149, 0x42683340, F=0x0 +0, 10, 10, 1, 6220800, 0xd4d0de99 +1, 10, 10, 1, 78, 0xad8a0c29, F=0x0 +0, 11, 11, 1, 6220800, 0x6f18e3e4 +1, 11, 11, 1, 72, 0x8c1c0caf, F=0x0 +0, 12, 12, 1, 6220800, 0xcfbbe357 +0, 13, 13, 1, 6220800, 0x70cfe436 diff --git a/tests/ref/fate/mxf-remux-xavc b/tests/ref/fate/mxf-remux-xavc new file mode 100644 index 0000000000..7b221208e5 --- /dev/null +++ b/tests/ref/fate/mxf-remux-xavc @@ -0,0 +1,71 @@ +d0fea4450c0f7c51b0af3631025dfa90 *tests/data/fate/mxf-remux-xavc.mxf +1436217 tests/data/fate/mxf-remux-xavc.mxf +#extradata 0: 97, 0xe2f818c1 +#tb 0: 1/25 +#media_type 0: video +#codec_id 0: h264 +#dimensions 0: 1920x1080 +#sar 0: 1/1 +#tb 1: 1/48000 +#media_type 1: audio +#codec_id 1: pcm_s24le +#sample_rate 1: 48000 +#channel_layout 1: 4 +#channel_layout_name 1: mono +#tb 2: 1/48000 +#media_type 2: audio +#codec_id 2: pcm_s24le +#sample_rate 2: 48000 +#channel_layout 2: 4 +#channel_layout_name 2: mono +#tb 3: 1/48000 +#media_type 3: audio +#codec_id 3: pcm_s24le +#sample_rate 3: 48000 +#channel_layout 3: 4 +#channel_layout_name 3: mono +#tb 4: 1/48000 +#media_type 4: audio +#codec_id 4: pcm_s24le +#sample_rate 4: 48000 +#channel_layout 4: 4 +#channel_layout_name 4: mono +#tb 5: 1/25 +#media_type 5: data +#codec_id 5: none +0, 0, 0, 1, 553248, 0xbda0217f +1, 0, 0, 1920, 5760, 0xd5894ac8 +2, 0, 0, 1920, 5760, 0x86314faa +3, 0, 0, 1920, 5760, 0x00000000 +4, 0, 0, 1920, 5760, 0x00000000 +5, 0, 0, 1, 150, 0xc6f20c7f +0, 1, 1, 1, 119985, 0x6ba5546e, F=0x0 +1, 1920, 1920, 1920, 5760, 0x9f71829c +2, 1920, 1920, 1920, 5760, 0x38049906 +3, 1920, 1920, 1920, 5760, 0x00000000 +4, 1920, 1920, 1920, 5760, 0x00000000 +5, 1, 1, 1, 150, 0xc6f20c7f +0, 2, 2, 1, 98815, 0xe47762ff, F=0x0 +0, 2, 3, 1, 257022, 0xd0647527, F=0x0 +1, 3840, 3840, 1920, 5760, 0xf7245421 +2, 3840, 3840, 1920, 5760, 0x5cd058b0 +3, 3840, 3840, 1920, 5760, 0x00000000 +4, 3840, 3840, 1920, 5760, 0x00000000 +5, 2, 2, 1, 150, 0xc6f20c7f +1, 5760, 5760, 1920, 5760, 0x3cef69d8 +2, 5760, 5760, 1920, 5760, 0x176daa39 +3, 5760, 5760, 1920, 5760, 0x00000000 +4, 5760, 5760, 1920, 5760, 0x00000000 +5, 3, 3, 1, 150, 0xc6f20c7f +0, 4, 4, 1, 123833, 0xff7b324e, F=0x0 +1, 7680, 7680, 1920, 5760, 0x6b647758 +2, 7680, 7680, 1920, 5760, 0xbf6f69de +3, 7680, 7680, 1920, 5760, 0x00000000 +4, 7680, 7680, 1920, 5760, 0x00000000 +5, 4, 4, 1, 150, 0xc6f20c7f +0, 5, 5, 1, 117099, 0x7d6aac45, F=0x0 +1, 9600, 9600, 1920, 5760, 0x0958976a +2, 9600, 9600, 1920, 5760, 0xe3957d5e +3, 9600, 9600, 1920, 5760, 0x00000000 +4, 9600, 9600, 1920, 5760, 0x00000000 +5, 5, 5, 1, 150, 0xc6f20c7f
These tests exhibit two bugs: Instead of using the in-band extradata the demuxer makes up some extradata designed for AVC intra tracks that lack in-band extradata; these files are nevertheless decodable because of the in-band extradata. Furthermore, the frame reordering is lost. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- tests/fate/mxf.mak | 22 ++++++++++- tests/ref/fate/mxf-remux-h264 | 37 ++++++++++++++++++ tests/ref/fate/mxf-remux-xavc | 71 +++++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 tests/ref/fate/mxf-remux-h264 create mode 100644 tests/ref/fate/mxf-remux-xavc