[FFmpeg-devel] [PATCH 1/2] avcodec/s302m: enable non-PCM decoding

[Andreas Rheinhardt]

Gyan Doshi:
> 
> 
> On 2024-01-25 06:47 pm, Andreas Rheinhardt wrote:
>> Gyan Doshi:
>>>
>>> On 2024-01-25 10:29 am, Andreas Rheinhardt wrote:
>>>> Gyan Doshi:
>>>>> Set up framework for non-PCM decoding in-place and
>>>>> add support for Dolby-E decoding.
>>>>>
>>>>> Useful for direct transcoding of non-PCM audio in live inputs.
>>>>> ---
>>>>>    configure          |   1 +
>>>>>    doc/decoders.texi  |  40 +++
>>>>>    libavcodec/s302m.c | 609
>>>>> +++++++++++++++++++++++++++++++++++++--------
>>>>>    3 files changed, 543 insertions(+), 107 deletions(-)
>>>>>
>>>>> diff --git a/configure b/configure
>>>>> index c8ae0a061d..8db3fa3f4b 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -2979,6 +2979,7 @@ rv20_decoder_select="h263_decoder"
>>>>>    rv20_encoder_select="h263_encoder"
>>>>>    rv30_decoder_select="golomb h264pred h264qpel mpegvideodec rv34dsp"
>>>>>    rv40_decoder_select="golomb h264pred h264qpel mpegvideodec rv34dsp"
>>>>> +s302m_decoder_select="dolby_e_decoder"
>>>>>    screenpresso_decoder_deps="zlib"
>>>>>    shorten_decoder_select="bswapdsp"
>>>>>    sipr_decoder_select="lsp"
>>>>> diff --git a/doc/decoders.texi b/doc/decoders.texi
>>>>> index 293c82c2ba..9f85c876bf 100644
>>>>> --- a/doc/decoders.texi
>>>>> +++ b/doc/decoders.texi
>>>>> @@ -347,6 +347,46 @@ configuration. You need to explicitly configure
>>>>> the build with
>>>>>    An FFmpeg native decoder for Opus exists, so users can decode Opus
>>>>>    without this library.
>>>>>    + at section s302m
>>>>> +
>>>>> +SMPTE ST 302 decoder.
>>>>> +
>>>>> +SMPTE ST 302 is a method for storing AES3 data format within an MPEG
>>>>> Transport
>>>>> +Stream. AES3 streams can contain LPCM streams of 2, 4, 6 or 8
>>>>> channels with a
>>>>> +bit depth of 16, 20 or 24-bits at a sample rate of 48 kHz.
>>>>> +They can also contain non-PCM codec streams such as AC-3 or Dolby-E.
>>>>> +
>>>> This sounds like we should add bitstream filters to extract the proper
>>>> underlying streams instead.
>>>> (I see only two problems with this approach: The BSF API needs to set
>>>> the CodecID of the output during init, but at this point no packet has
>>>> reached the BSF to determine it. And changing codec IDs mid-stream is
>>>> also not supported.)
>>> In theory, this decoder shouldn't exist, as it is just a carrier,
>>> whether of LPCM or non-PCM.
>>> FFmpeg architecture also imposes a fundamental limitation in that one
>>> s302m stream may
>>> carry multiple payload streams and we support only one decoding context
>>> per input stream
>> Then why does the demuxer not separate the data into multiple streams?
> 
> I didn't add demuxing support for this codec in MPEGTS, but I can venture
> 
> a) it would mean essentially inlining this decoder in the demuxer.
> b) it would lead to a mapping of 1 actual to N virtual streams. How
> would stream_ids be assigned?
> c) this codec specifies for multiple payloads but I haven't seen an
> actual sample
> 
> 
>>>>> +
>>>>> +    ret = init_get_bits8(&gb, buf, buf_size);
>>>>> +    if (ret < 0)
>>>>> +        return ret;
>>>>> +
>>>>> +    aes_frm_size = (s->bits + 4) * 2 / 8;
>>>>> +    if (buf_size < aes_frm_size * 2)  // not enough to contain
>>>>> data_type & length_code
>>>>> +        return AVERROR_INVALIDDATA;
>>>>> +
>>>>> +    state = get_bits64(&gb, aes_frm_size * 8);
>>>>> +
>>>>> +    while (!IS_NONPCMSYNC(s->bits,state) && (get_bits_left(&gb) >=
>>>>> 8))
>>>>> +        state = (state << 8) | get_bits(&gb, 8);
>>>> Reading byte-aligned data with a GetBit context is very suboptimal.
>>> What is the performance difference vs. uint8 pointers?
>> Typically worse by a constant factor.
> 
> But in this scenario (of a few dozen bytes per packet if dolby_e), how
> significant is it?
> 

You are only looking at the ordinary case and not fuzzing samples where
it is more than a few dozen bytes.

- Andreas