[FFmpeg-devel] [PATCH v2 3/5] avformat/s337m: Make available as subdemuxer

[Tomas Härdin]

tis 2019-08-06 klockan 13:50 +0200 skrev Nicolas Gaullier:
> +int avpriv_s337m_probe_stream(void *avc, AVIOContext *pb, AVStream
> **st, int size)
> +{
> +    if ( size >= S337M_MIN_OFFSET &&
> +        ((*st)->codecpar->codec_id == AV_CODEC_ID_PCM_S16LE ||
> (*st)->codecpar->codec_id == AV_CODEC_ID_PCM_S24LE) &&
> +               (*st)->codecpar->channels == 2) {
> +        uint8_t *buf;
> +        int64_t data_offset;
> +        int pos = 0;
> +        double s337m_phase;
> +
> +        size = FFMIN(size, S337M_MAX_OFFSET);
> +        if (!(buf = av_mallocz(size)))
> +            return AVERROR(ENOMEM);

braces missing here

> +        data_offset = avio_tell(pb);
> +        if (avio_read(pb, buf, size) != size) {
> +            av_freep(&buf);
> +            return AVERROR(EIO);
> +        }
> +        avio_seek(pb, data_offset, SEEK_SET);
> +
> +        while (pos < size - 9 && !buf[pos])
> +            pos++;

and here

> +        if (pos < size - 9 &&
> +            (s337m_phase = (double)pos * 4 / (*st)->codecpar-
> >bits_per_coded_sample / (*st)->codecpar->sample_rate) >=
> S337M_PHASE_PROBE_MIN) {

Can this be turned into an integer-only test?
I'd put the assignment outside the if since it's a bit involved.

> +#ifndef AVFORMAT_S337M_H
> +#define AVFORMAT_S337M_H
> +
> +#define MARKER_16LE         0x72F81F4E
> +#define MARKER_20LE         0x20876FF0E154
> +#define MARKER_24LE         0x72F8961F4EA5
> +
> +#define IS_16LE_MARKER(state)   ((state & 0xFFFFFFFF) ==
> MARKER_16LE)
> +#define IS_20LE_MARKER(state)   ((state & 0xF0FFFFF0FFFF) ==
> MARKER_20LE)
> +#define IS_24LE_MARKER(state)   ((state & 0xFFFFFFFFFFFF) ==
> MARKER_24LE)
> +#define IS_LE_MARKER(state)     (IS_16LE_MARKER(state) ||
> IS_20LE_MARKER(state) || IS_24LE_MARKER(state))
> +
> +#define S337M_MIN_OFFSET 1601*4
> +#define S337M_MAX_OFFSET 2002*6
> +
> +#define S337M_PHASE_PROBE_MIN   0.000000
> +#define DOLBY_E_PHASE_MIN       0.000610
> +#define DOLBY_E_PHASE_MAX       0.001050

These macros should probably have an AVPRIV_ prefix or similar,
assuming they're visible to users (not sure)

/Tomas