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

[Tomas Härdin]

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 at 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