[FFmpeg-devel] [PATCH 0/7] RFC: complete rework of s337m support

[ffnicolasg at sfr.fr]

From: Nicolas Gaullier <nicolas.gaullier at cji.paris>

I would like to have some feedback on this work before going on...
I send this as an RFC, and so, I have not considered versioning,
changelog, doc, deprecation schemes. More testing and testing by
other people is also obviously required.

The fate samples are available here: https://0x0.st/X7mV.zip

To begin, I'd like to raise the issues in current code:
- current code requires a constant 'offset' between each frame, but
this requirement is not standard and prevents conformant files to be
decoded. It also prevents support for unexpected forms of Dolby E
(Dolby E D2 is not supported because its 'offset' is not recognised).
- current code does not allow 'missing frames' with silence fallback.
However, this is very commonplace: for example, someone wants to add
a still picture with credits and no audio, so inserts some null bytes
= pcm/silence. The standard recommands to be able to switch between pcm
and Dolby E frame by frame, so it 'should' be supported.
- the Dolby E sample rate is not always an integer, and its rounded
value makes its sync slightly drift.
- spdif and s337m are implemented separately, despite it's almost the
same thing; the spdif syncword is even the same as the 16-bit s337m
syncword!

At the end, it really seems unreasonable to handle raw dolby_e streams
without the upper s337m layer. The native dolby_e decoder output with
its weird sample rate (specially @29.97) is not very convenient, too.

Here is what I designed:
- the mpegts/wav/mxf(stereo) support is handled via stream probing which
is a proven feature for years, with very little code addition and low
dependance to the demuxers. It is the spirit of the standard: every
uncompressed stereo audio can support s337m.
- a s337m parser: full parsing is required even for frame-wrapped mxf
as broken files that do not have a valid s337m phase results in audio
frames crossing the edit unit boundaries.
- a s337m decoder: it includes a resampler: output and input sample_rate
are the same, sync is always correct. It would be possible to implement
a full pcm fallback, but currently only a silence/pcm fallback is
provided. A 'passthrough' option is also provided and would make it
possible to mux again into wav, mxf or whatever. I guess one could
imagine a bitstream filter to fix the s337m phase to a clean, fixed
offset value (as expected by the current s337m demuxer for example).
- the demuxer is split in two raw demuxers: a 16-bit and a 24-bit. All
the logic remains in avcodec.
- mpegts: the s302m decoder is embedded in the mpegts demuxer; it does
not care about s337m as everything relies on ffmpeg's stream probing.

What could be done further: I am not used to working with spdif,
but I guess much of the code and design should be shared with s337m.

Tricks:
I really have no idea what a migration path could be for users and
developpers, notably the "non_pcm_data" option of mpegts seems
pretty 'unmappable'...

Please comment!

Thank you
Nicolas

Nicolas Gaullier (7):
  avcodec: add s337m parser and decoder
  avformat/spdif,s337m: use shared code from avcodec
  avformat/s337m: switch to two rawdec for 16- and 24-bit
  fate/acodec: add 20/24bit s302m tests
  avformat/mpegts: add s337m support
  avformat: add s337m support in mpegts, wav and mxf stereo tracks
  fate: add s337m decode and demux tests

 configure                                |   2 +
 libavcodec/Makefile                      |   4 +
 libavcodec/allcodecs.c                   |   2 +
 libavcodec/codec_desc.c                  |  14 +
 libavcodec/codec_id.h                    |   2 +
 libavcodec/dolby_e_parse.c               |   3 +
 libavcodec/parsers.c                     |   2 +
 libavcodec/s337m.c                       | 327 +++++++++++++++++++++++
 libavcodec/s337m_parser.c                | 133 +++++++++
 libavcodec/spdif_s337m_parse.c           | 142 ++++++++++
 libavcodec/spdif_s337m_parser_internal.h |  92 +++++++
 libavcodec/utils.c                       |   2 +
 libavcodec/version.c                     |   2 +-
 libavformat/Makefile                     |   7 +-
 libavformat/allformats.c                 |   3 +-
 libavformat/demux.c                      |   2 +
 libavformat/mpegts.c                     | 138 +++++++++-
 libavformat/mxfdec.c                     |   9 +-
 libavformat/s337m.c                      | 217 +++++----------
 libavformat/spdif.c                      |  42 ---
 libavformat/spdif.h                      |   1 -
 libavformat/spdifdec.c                   |   3 +-
 libavformat/spdifenc.c                   |   3 +-
 libavformat/wavdec.c                     |   3 +-
 tests/fate/acodec.mak                    |  16 +-
 tests/fate/audio.mak                     |  15 +-
 tests/fate/demux.mak                     |  15 +-
 tests/ref/acodec/s302m                   |   4 -
 tests/ref/acodec/s302m-16bit             |   4 +
 tests/ref/acodec/s302m-20bit             |   4 +
 tests/ref/acodec/s302m-24bit             |   4 +
 tests/ref/fate/s337m-demux               |  30 ---
 tests/ref/fate/s337m-demux-mxf           |  56 ++++
 tests/ref/fate/s337m-demux-raw           |  30 +++
 tests/ref/fate/s337m-demux-ts-20         |  44 +++
 tests/ref/fate/s337m-demux-ts-24         |  39 +++
 tests/ref/fate/s337m-demux-wav           |  11 +
 tests/ref/fate/s337m-demux-wav-miss1-3-5 |   8 +
 38 files changed, 1185 insertions(+), 250 deletions(-)
 create mode 100644 libavcodec/s337m.c
 create mode 100644 libavcodec/s337m_parser.c
 create mode 100644 libavcodec/spdif_s337m_parse.c
 create mode 100644 libavcodec/spdif_s337m_parser_internal.h
 delete mode 100644 libavformat/spdif.c
 delete mode 100644 tests/ref/acodec/s302m
 create mode 100644 tests/ref/acodec/s302m-16bit
 create mode 100644 tests/ref/acodec/s302m-20bit
 create mode 100644 tests/ref/acodec/s302m-24bit
 delete mode 100644 tests/ref/fate/s337m-demux
 create mode 100644 tests/ref/fate/s337m-demux-mxf
 create mode 100644 tests/ref/fate/s337m-demux-raw
 create mode 100644 tests/ref/fate/s337m-demux-ts-20
 create mode 100644 tests/ref/fate/s337m-demux-ts-24
 create mode 100644 tests/ref/fate/s337m-demux-wav
 create mode 100644 tests/ref/fate/s337m-demux-wav-miss1-3-5

-- 
2.30.2