[FFmpeg-devel,v3,4/6] avformat/wavdec: s337m support

Enabled by AVOption dolbyeprobe
Requires stereo
---
 libavformat/s337m.h  |  1 +
 libavformat/wavdec.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

tis 2019-08-06 klockan 17:08 +0200 skrev Nicolas Gaullier:
> Enabled by AVOption dolbyeprobe
> Requires stereo
> ---
>  libavformat/s337m.h  |  1 +
>  libavformat/wavdec.c | 23 +++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/libavformat/s337m.h b/libavformat/s337m.h
> index 0f21a23a30..aff76373d3 100644
> --- a/libavformat/s337m.h
> +++ b/libavformat/s337m.h
> @@ -32,6 +32,7 @@
>  
>  #define S337M_MIN_OFFSET 1601*4
>  #define S337M_MAX_OFFSET 2002*6
> +#define AVPRIV_S337M_MAX_RECOMMENDED_PROBE_SIZE 2*S337M_MAX_OFFSET
>  
>  #define S337M_PROBE_GUARDBAND_MIN_BYTES     0
>  #define DOLBY_E_PHASE_MIN       0.000610
> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
> index 52194f54ef..2ef2d9e549 100644
> --- a/libavformat/wavdec.c
> +++ b/libavformat/wavdec.c
> @@ -41,6 +41,9 @@
>  #include "riff.h"
>  #include "w64.h"
>  #include "spdif.h"
> +#if CONFIG_S337M_DEMUXER
> +#include "s337m.h"
> +#endif
>  
>  typedef struct WAVDemuxContext {
>      const AVClass *class;
> @@ -59,6 +62,9 @@ typedef struct WAVDemuxContext {
>      int smv_given_first;
>      int unaligned; // e.g. if an odd number of bytes ID3 tag was
> prepended
>      int rifx; // RIFX: integer byte order for parameters is big
> endian
> +#if CONFIG_S337M_DEMUXER
> +    int dolby_e_probe;
> +#endif
>  } WAVDemuxContext;
>  
>  static void set_spdif(AVFormatContext *s, WAVDemuxContext *wav)
> @@ -593,6 +599,10 @@ break_loop:
>      } else if (st->codecpar->codec_id == AV_CODEC_ID_ADPCM_MS && st-
> >codecpar->channels > 2) {
>          st->codecpar->block_align *= st->codecpar->channels;
>      }
> +#if CONFIG_S337M_DEMUXER
> +    if (wav->dolby_e_probe)
> +        avpriv_s337m_probe_stream((void *)s, s->pb, &st,
> FFMIN(AVPRIV_S337M_MAX_RECOMMENDED_PROBE_SIZE, wav->data_end -
> avio_tell(pb)));

Missing braces

> +#endif
>  
>      ff_metadata_conv_ctx(s, NULL, wav_metadata_conv);
>      ff_metadata_conv_ctx(s, NULL, ff_riff_info_conv);
> @@ -701,6 +711,7 @@ smv_out:
>          wav->data_end = avio_tell(s->pb) + left;
>      }
>  
> +    if (!CONFIG_S337M_DEMUXER || st->codecpar->codec_id != 
> AV_CODEC_ID_DOLBY_E) {

It seems you missed the other part of my comment on this. Keeping the
#if is fine, especially considering..

>      size = MAX_SIZE;
>      if (st->codecpar->block_align > 1) {
>          if (size < st->codecpar->block_align)
> @@ -709,6 +720,11 @@ smv_out:
>      }
>      size = FFMIN(size, left);
>      ret  = av_get_packet(s->pb, pkt, size);
> +    } else {
> +        size = FFMIN(AVPRIV_S337M_MAX_RECOMMENDED_PROBE_SIZE, left);

.. this won't compile if S377m is disabled. Just go with the old #if
solution, sorry for causing confusion

> +        ret  = avpriv_s337m_get_packet((void *)s, s->pb, pkt, size,
> 0);
> +    }
> +
>      if (ret < 0)
>          return ret;
>      pkt->stream_index = 0;
> @@ -754,6 +770,9 @@ static int wav_read_seek(AVFormatContext *s,
>  #define DEC AV_OPT_FLAG_DECODING_PARAM
>  static const AVOption demux_options[] = {
>      { "ignore_length", "Ignore length", OFFSET(ignore_length),
> AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, DEC },
> +#if CONFIG_S337M_DEMUXER
> +    {"dolbyeprobe", "Probe Dolby E in pcm/stereo streams",
> OFFSET(dolby_e_probe), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, DEC },
> +#endif
>      { NULL },
>  };
>  
> @@ -895,6 +914,10 @@ static int w64_read_header(AVFormatContext *s)
>      st->need_parsing = AVSTREAM_PARSE_FULL_RAW;
>  
>      avio_seek(pb, data_ofs, SEEK_SET);
> +#if CONFIG_S337M_DEMUXER
> +    if (wav->dolby_e_probe)
> +        avpriv_s337m_probe_stream((void *)s, s->pb, &st,
> FFMIN(AVPRIV_S337M_MAX_RECOMMENDED_PROBE_SIZE, wav->data_end -
> avio_tell(pb)));

Braces here too

/Tomas

>.. this won't compile if S377m is disabled. Just go with the old #if

>solution, sorry for causing confusion

My mistake but I think it is a good idea to get rid of some bulky #if... Do you think a mixed usage of if and #if would be acceptable ? I think the following would be the best :
    if (!CONFIG_S337M_DEMUXER || st->codecpar->codec_id != AV_CODEC_ID_DOLBY_E) {
(...)
#if CONFIG_S337M_DEMUXER
    } else {
(... using s337m_get_packet method ... )
#endif
    }

NB: I have to rework all the patchset with carl's feedback, but this point will still remain, so better to have it solved now.

Thank you
Nicolas