diff mbox series

[FFmpeg-devel,03/17] fate/mxf: Add tests for H.264 remuxing

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
Related show

Checks

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

Commit Message

Andreas Rheinhardt Nov. 9, 2021, 6:01 p.m. UTC
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

Comments

Tomas Härdin Nov. 9, 2021, 10:42 p.m. UTC | #1
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
Andreas Rheinhardt Nov. 10, 2021, 1:52 p.m. UTC | #2
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
Tomas Härdin Nov. 10, 2021, 3:14 p.m. UTC | #3
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 mbox series

Patch

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