diff mbox series

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

Message ID 20230213180936.815-4-nicolas.gaullier@cji.paris
State New
Headers show
Series wavdev: s337m support | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Nicolas Gaullier Feb. 13, 2023, 6:09 p.m. UTC
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(+)

Comments

Tomas Härdin Feb. 16, 2023, 10:36 a.m. UTC | #1
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
Nicolas Gaullier Feb. 17, 2023, 10:12 a.m. UTC | #2
> 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
Tomas Härdin Feb. 21, 2023, 9:41 a.m. UTC | #3
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
Nicolas Gaullier Feb. 21, 2023, 10:57 a.m. UTC | #4
>> 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
diff mbox series

Patch

diff --git a/libavformat/s337m.c b/libavformat/s337m.c
index 2a566e3cba..9d2ac265b6 100644
--- a/libavformat/s337m.c
+++ b/libavformat/s337m.c
@@ -135,6 +135,38 @@  static int s337m_probe(const AVProbeData *p)
     return 0;
 }
 
+int ff_s337m_probe(const uint8_t *buf, int size, enum AVCodecID *codec, int container_word_bits)
+{
+    int pos = 0;
+    int consecutive_codes = 0;
+
+    if ( size < S337M_MIN_OFFSET)
+        return 0;
+    size = FFMIN(3 * S337M_MAX_OFFSET, size);
+    if (container_word_bits != 16 && container_word_bits != 24)
+        return AVERROR_INVALIDDATA;
+
+    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;
+}
+
 static int s337m_read_header(AVFormatContext *s)
 {
     s->ctx_flags |= AVFMTCTX_NOHEADER;
diff --git a/libavformat/s337m.h b/libavformat/s337m.h
index af2c4c85a3..94e79dce5d 100644
--- a/libavformat/s337m.h
+++ b/libavformat/s337m.h
@@ -21,6 +21,22 @@ 
 #ifndef AVFORMAT_S337M_H
 #define AVFORMAT_S337M_H
 
+#define S337M_MIN_OFFSET 1601*4
+#define S337M_MAX_OFFSET 2002*6
+
+#define S337M_PROBE_GUARDBAND_MIN_BYTES     0
+
+/**
+ * Detect s337m packets in a PCM_S16LE/S24LE stereo stream
+ * Requires 3 samples with enough (S337M_PROBE_GUARDBAND_MIN_BYTES) and clean (set to zero) guard band
+ * @param p_buf Buffer
+ * @param size Buffer size
+ * @param codec Returns AV_CODEC_ID_DOLBY_E upon successful probing
+ * @param container_word_bits 16 or 24
+ * @return = AVPROBE_SCORE
+ */
+int ff_s337m_probe(const uint8_t *p_buf, int size, enum AVCodecID *codec, int container_word_bits);
+
 /**
  * Read s337m packets in a PCM_S16LE/S24LE stereo stream
  * Returns the first inner packet found