Message ID | 20230213180936.815-4-nicolas.gaullier@cji.paris |
---|---|
State | New |
Headers | show |
Series | wavdev: s337m support | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
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
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