From: Nicolas Gaullier <nicolas.gaullier@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
