[FFmpeg-devel,4/9] avformat/s337m: New ff_s337m_probe()

Similar to ff_spdif_probe() with two additionnal parameters:
- an AVClass for logging
- the bit resolution of the container as it may be 16 or 24 for s337m
---
 libavformat/s337m.c | 35 +++++++++++++++++++++++++++++++++++
 libavformat/s337m.h | 19 +++++++++++++++++++
 2 files changed, 54 insertions(+)

fre 2020-01-03 klockan 16:56 +0100 skrev Nicolas Gaullier:
> Similar to ff_spdif_probe() with two additionnal parameters:
> - an AVClass for logging
> - the bit resolution of the container as it may be 16 or 24 for s337m
> ---
>  libavformat/s337m.c | 35 +++++++++++++++++++++++++++++++++++
>  libavformat/s337m.h | 19 +++++++++++++++++++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/libavformat/s337m.c b/libavformat/s337m.c
> index 5c8ab2649c..dc62d6ab98 100644
> --- a/libavformat/s337m.c
> +++ b/libavformat/s337m.c
> @@ -133,6 +133,41 @@ static int s337m_probe(const AVProbeData *p)
>      return 0;
>  }
>  
> +int ff_s337m_probe(const uint8_t *buf, int size, enum AVCodecID *codec, void *avc, int container_word_bits)
> +{
> +    int pos = 0;
> +    int consecutive_codes = 0;
> +
> +    if ( size < S337M_MIN_OFFSET)
> +        return 0;
> +    size = FFMIN(2 * S337M_MAX_OFFSET, size);
> +
> +    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)
> +            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(avc, state, data_type, data_size, container_word_bits, &offset, codec))
> +            return 0;

Looks OK


> +        if (avc) {
> +            double s337m_phase = pos * 4. / container_word_bits / 48000;
> +            av_log(avc, AV_LOG_INFO, "s337m sample %d detected with phase = %.6fs\n", consecutive_codes, s337m_phase);
> +            if (*codec == AV_CODEC_ID_DOLBY_E && (s337m_phase < DOLBY_E_PHASE_MIN || s337m_phase > DOLBY_E_PHASE_MAX))
> +                    av_log(avc, AV_LOG_WARNING, "Dolby E phase is out of valid range (%.6fs-%.6fs)\n", DOLBY_E_PHASE_MIN, DOLBY_E_PHASE_MAX);

Still not a huge fan of this "phase" notion here, but I suppose it's
nothing to get hung up about

/Tomas

Am Fr., 3. Jan. 2020 um 16:57 Uhr schrieb Nicolas Gaullier
<nicolas.gaullier@cji.paris>:
>
> Similar to ff_spdif_probe() with two additionnal parameters:
> - an AVClass for logging
> - the bit resolution of the container as it may be 16 or 24 for s337m
> ---
>  libavformat/s337m.c | 35 +++++++++++++++++++++++++++++++++++
>  libavformat/s337m.h | 19 +++++++++++++++++++
>  2 files changed, 54 insertions(+)
>
> diff --git a/libavformat/s337m.c b/libavformat/s337m.c
> index 5c8ab2649c..dc62d6ab98 100644
> --- a/libavformat/s337m.c
> +++ b/libavformat/s337m.c
> @@ -133,6 +133,41 @@ static int s337m_probe(const AVProbeData *p)
>      return 0;
>  }
>
> +int ff_s337m_probe(const uint8_t *buf, int size, enum AVCodecID *codec, void *avc, int container_word_bits)
> +{
> +    int pos = 0;
> +    int consecutive_codes = 0;
> +
> +    if ( size < S337M_MIN_OFFSET)
> +        return 0;
> +    size = FFMIN(2 * S337M_MAX_OFFSET, size);
> +
> +    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)
> +            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(avc, state, data_type, data_size, container_word_bits, &offset, codec))
> +            return 0;
> +        if (avc) {
> +            double s337m_phase = pos * 4. / container_word_bits / 48000;
> +            av_log(avc, AV_LOG_INFO, "s337m sample %d detected with phase = %.6fs\n", consecutive_codes, s337m_phase);
> +            if (*codec == AV_CODEC_ID_DOLBY_E && (s337m_phase < DOLBY_E_PHASE_MIN || s337m_phase > DOLBY_E_PHASE_MAX))
> +                    av_log(avc, AV_LOG_WARNING, "Dolby E phase is out of valid range (%.6fs-%.6fs)\n", DOLBY_E_PHASE_MIN, DOLBY_E_PHASE_MAX);
> +        }
> +    } while (++consecutive_codes < 2);
> +
> +    return AVPROBE_SCORE_MAX;
> +}

Probe functions must never av_log() anything.

Since you add an option in a later patch: Can you explain the
reasoning for the whole patchset better?
If DolbyE can be auto-detected (I assume so), this should be
done and no further option should be needed.

Carl Eugen

>Since you add an option in a later patch: Can you explain the
>reasoning for the whole patchset better?
>If DolbyE can be auto-detected (I assume so), this should be
>done and no further option should be needed.
>
>Carl Eugen

A common use case is stream copying : you want to be able to forward the input dolbyE to the output. But the mxf mux, for example, does not support dolby_e (up to now). In the future, if we come to support muxing dolby_e in s337m, then yes, that would be nice, we would be able to remux a dolby_e and fix its position/guard band, that would be a very interesting repairing workflow, but that would require much hard work (DolbyE recommanded position is dependant on video raster etc.).
Second thing is that, even if we were able to support stream copying, the fact is that users have tons of script like this :
ffmpeg -i input.mxf -vcodec copy output.mxf
I mean broadcasters are often missing the "audio codec copy" parameter because they think "my audio is uncompressed 16 bits, stream copying is bitexact the same as transcoding". I am afraid I have myself many scripts like this...
So, it is necessary to default to not-probe/decode dolby_e to not break scripts.
And most of the time, the fact is that users just want to pass trough the data as they indeed need to be agnostic.
This was the reason to choose to 1) add an AVOption, and 2) makes it default to disabled

Nicolas