[FFmpeg-devel,3/6] avformat/s337m: New ff_s337m_probe()

Similar to ff_spdif_probe() with just an additional checking of
the bit resolution of the container as it may be 16 or 24 for s337m.
---
 libavformat/s337m.c | 32 ++++++++++++++++++++++++++++++++
 libavformat/s337m.h | 16 ++++++++++++++++
 2 files changed, 48 insertions(+)

mån 2023-02-13 klockan 19:09 +0100 skrev Nicolas Gaullier:
> +    do {
> +        uint64_t state;
> +        int data_type, data_size, offset;
> +        while (pos < size - 12 && !buf[pos]) {
> +            pos++;
> +        }
> +        if (pos >= size - 12 || pos <
> S337M_PROBE_GUARDBAND_MIN_BYTES || pos % (container_word_bits == 16 ?
> 4 : 6))
> +            return 0;
> +        state = container_word_bits == 16 ? AV_RB32(buf + pos) :
> AV_RB48(buf + pos);
> +        if (!IS_LE_MARKER(state))
> +            return 0;
> +        data_type = container_word_bits == 16 ? AV_RL16(buf + pos +
> 4) : AV_RL24(buf + pos + 6);
> +        data_size = container_word_bits == 16 ? AV_RL16(buf + pos +
> 6) : AV_RL24(buf + pos + 9);
> +        if (s337m_get_offset_and_codec(NULL, state, data_type,
> data_size, container_word_bits, &offset, codec))
> +            return 0;
> +        pos = ++consecutive_codes * (offset + 4*(container_word_bits
> == 16 ? 4 : 6));
> +    } while (consecutive_codes < 3);
> +
> +    return AVPROBE_SCORE_MAX;
> +}

The logic here is a bit hairy and I don't have time atm to digest it,
but is it entirely contained in S337m or would one need to read other
specs too?

/Tomas

> The logic here is a bit hairy and I don't have time atm to digest it, but is it entirely contained in S337m or would one need to read other specs too?
>
>/Tomas

ff_s337m_probe is very similar to s337m_probe: what mainly differs is the input parameters.
The one little thing I added is the S337M_PROBE_GUARDBAND_MIN_BYTES.
Currently it is set to 0, so has no effect (and of course I can remove it if someone object).
There is two things to know about it:
- one is that some DolbyE decoder implementations does not support the s337m sync word to be the first word,
A minimal guard band (full of zero) is required in such a case : 1 word is enough in the cases I experimented.
One developer might find it useful to set S337M_PROBE_GUARDBAND_MIN_BYTES to 1 in order to ffprobe-qc/reject such files.
- one other thing is that, currently, the detection is based on 3 consecutive samples,
But there are other implementations in the wild. A common single-sample implementation is to simply require
a sufficient S337M_PROBE_GUARDBAND_MIN_BYTES in order to avoid a fake detection.
(for 16 bits, this is really dangerous!; for 24 bits, I think it is fair but would still require some little additions to be 100% sure).

Nicolas

fre 2023-02-17 klockan 10:12 +0000 skrev Nicolas Gaullier:
> > The logic here is a bit hairy and I don't have time atm to digest
> > it, but is it entirely contained in S337m or would one need to read
> > other specs too?
> > 
> > /Tomas
> 
> ff_s337m_probe is very similar to s337m_probe: what mainly differs is
> the input parameters.
> The one little thing I added is the S337M_PROBE_GUARDBAND_MIN_BYTES.
> Currently it is set to 0, so has no effect (and of course I can
> remove it if someone object).
> There is two things to know about it:
> - one is that some DolbyE decoder implementations does not support
> the s337m sync word to be the first word,
> A minimal guard band (full of zero) is required in such a case : 1
> word is enough in the cases I experimented.
> One developer might find it useful to set
> S337M_PROBE_GUARDBAND_MIN_BYTES to 1 in order to ffprobe-qc/reject
> such files.
> - one other thing is that, currently, the detection is based on 3
> consecutive samples,
> But there are other implementations in the wild. A common single-
> sample implementation is to simply require
> a sufficient S337M_PROBE_GUARDBAND_MIN_BYTES in order to avoid a fake
> detection.
> (for 16 bits, this is really dangerous!; for 24 bits, I think it is
> fair but would still require some little additions to be 100% sure).

What a mess.. And there's no other way to reliably probe this?

/Tomas

>> ff_s337m_probe is very similar to s337m_probe: what mainly differs is 
>> the input parameters.
>> The one little thing I added is the S337M_PROBE_GUARDBAND_MIN_BYTES.
>> Currently it is set to 0, so has no effect (and of course I can remove 
>> it if someone object).
>> There is two things to know about it:
>> - one is that some DolbyE decoder implementations does not support the 
>> s337m sync word to be the first word, A minimal guard band (full of 
>> zero) is required in such a case : 1 word is enough in the cases I 
>> experimented.
>> One developer might find it useful to set 
>> S337M_PROBE_GUARDBAND_MIN_BYTES to 1 in order to ffprobe-qc/reject 
>> such files.
>> - one other thing is that, currently, the detection is based on 3 
>> consecutive samples, But there are other implementations in the wild. 
>> A common single- sample implementation is to simply require a 
>> sufficient S337M_PROBE_GUARDBAND_MIN_BYTES in order to avoid a fake 
>> detection.
>> (for 16 bits, this is really dangerous!; for 24 bits, I think it is 
>> fair but would still require some little additions to be 100% sure).
>
>What a mess.. And there's no other way to reliably probe this?
>
>/Tomas

No! But the statistics are such that at some time, a fake probe is not "possible".
Anyway, we already have stream probings in current code, not only considering wav.
I think my patch is similar to the spdif probe and safe.

Nicolas