diff mbox

[FFmpeg-devel,2/3] avformat: Support s337m in mxf/wav/w64

Message ID 20190726164517.8428-3-nicolas.gaullier@arkena.com
State New
Headers show

Commit Message

Gaullier Nicolas July 26, 2019, 4:45 p.m. UTC
---
 libavformat/avformat.h      |  7 +++++++
 libavformat/mxfdec.c        | 20 +++++++++++++++++++-
 libavformat/options_table.h |  1 +
 libavformat/wavdec.c        |  7 ++++++-
 4 files changed, 33 insertions(+), 2 deletions(-)

Comments

Paul B Mahol July 26, 2019, 4:51 p.m. UTC | #1
On Fri, Jul 26, 2019 at 6:45 PM Nicolas Gaullier <
nicolas.gaullier@arkena.com> wrote:

> ---
>  libavformat/avformat.h      |  7 +++++++
>  libavformat/mxfdec.c        | 20 +++++++++++++++++++-
>  libavformat/options_table.h |  1 +
>  libavformat/wavdec.c        |  7 ++++++-
>  4 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 6eb329f13f..42bb094d81 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1951,6 +1951,13 @@ typedef struct AVFormatContext {
>       * - decoding: set by user
>       */
>      int skip_estimate_duration_from_pts;
> +
> +    /**
> +     * Probe dolby_e in PCM streams
> +     * - encoding: unused
> +     * - decoding: set by user
> +     */
> +    int dolby_e_probe;
>

Unacceptable addition. Use AVOptions.



>  } AVFormatContext;
>
>  #if FF_API_FORMAT_GET_SET
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index bb72fb9841..5b6eb9d756 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -56,6 +56,7 @@
>  #include "avformat.h"
>  #include "internal.h"
>  #include "mxf.h"
> +#include "s337m.h"
>
>  #define MXF_MAX_CHUNK_SIZE (32 << 20)
>
> @@ -302,6 +303,8 @@ typedef struct MXFMetadataReadTableEntry {
>      enum MXFMetadataSetType type;
>  } MXFMetadataReadTableEntry;
>
> +static int mxf_read_packet_init = 0;
> +static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt);
>  static int mxf_read_close(AVFormatContext *s);
>
>  /* partial keys to match */
> @@ -3247,6 +3250,19 @@ static int mxf_read_header(AVFormatContext *s)
>      if ((ret = mxf_parse_structural_metadata(mxf)) < 0)
>          goto fail;
>
> +    if (mxf->fc->dolby_e_probe)
> +    {
> +        int i;
> +        AVPacket *pkt = av_packet_alloc();
> +        av_init_packet(pkt);
> +        mxf_read_packet_init = 1;
> +        for (i = 0; i < mxf->fc->nb_streams; i++)
> +            mxf_read_packet(mxf->fc, pkt);
> +        mxf_read_packet_init = 0;
> +        av_freep(pkt);
> +        avio_seek(s->pb, essence_offset, SEEK_SET);
> +    }
> +
>      for (int i = 0; i < s->nb_streams; i++)
>          mxf_handle_missing_index_segment(mxf, s->streams[i]);
>
> @@ -3539,7 +3555,9 @@ static int mxf_read_packet(AVFormatContext *s,
> AVPacket *pkt)
>                      return ret;
>                  }
>              } else {
> -                ret = av_get_packet(s->pb, pkt, klv.length);
> +                if (mxf_read_packet_init)
> +                    s337m_probe_stream(mxf->fc, &st);
> +                ret = st->codecpar->codec_id == AV_CODEC_ID_DOLBY_E ?
> s337m_read_packet(s, pkt) : av_get_packet(s->pb, pkt, klv.length);
>                  if (ret < 0) {
>                      mxf->current_klv_data = (KLVPacket){{0}};
>                      return ret;
> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> index f2f077b34f..7f3c22d6bb 100644
> --- a/libavformat/options_table.h
> +++ b/libavformat/options_table.h
> @@ -111,6 +111,7 @@ static const AVOption avformat_options[] = {
>  {"protocol_blacklist", "List of protocols that are not allowed to be
> used", OFFSET(protocol_blacklist), AV_OPT_TYPE_STRING, { .str = NULL },
> CHAR_MIN, CHAR_MAX, D },
>  {"max_streams", "maximum number of streams", OFFSET(max_streams),
> AV_OPT_TYPE_INT, { .i64 = 1000 }, 0, INT_MAX, D },
>  {"skip_estimate_duration_from_pts", "skip duration calculation in
> estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts),
> AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D},
> +{"dolbyeprobe", "probe dolby_e in PCM streams", OFFSET(dolby_e_probe),
> AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, D},
>  {NULL},
>  };
>
> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
> index 52194f54ef..501c21f220 100644
> --- a/libavformat/wavdec.c
> +++ b/libavformat/wavdec.c
> @@ -41,6 +41,7 @@
>  #include "riff.h"
>  #include "w64.h"
>  #include "spdif.h"
> +#include "s337m.h"
>
>  typedef struct WAVDemuxContext {
>      const AVClass *class;
> @@ -593,6 +594,8 @@ 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 (s->dolby_e_probe)
> +        s337m_probe_stream(s, &st);
>
>      ff_metadata_conv_ctx(s, NULL, wav_metadata_conv);
>      ff_metadata_conv_ctx(s, NULL, ff_riff_info_conv);
> @@ -708,7 +711,7 @@ smv_out:
>          size = (size / st->codecpar->block_align) *
> st->codecpar->block_align;
>      }
>      size = FFMIN(size, left);
> -    ret  = av_get_packet(s->pb, pkt, size);
> +    ret  = st->codecpar->codec_id == AV_CODEC_ID_DOLBY_E ?
> s337m_read_packet(s, pkt) : av_get_packet(s->pb, pkt, size);
>      if (ret < 0)
>          return ret;
>      pkt->stream_index = 0;
> @@ -895,6 +898,8 @@ static int w64_read_header(AVFormatContext *s)
>      st->need_parsing = AVSTREAM_PARSE_FULL_RAW;
>
>      avio_seek(pb, data_ofs, SEEK_SET);
> +    if (s->dolby_e_probe)
> +        s337m_probe_stream(s, &st);
>
>      set_spdif(s, wav);
>
> --
> 2.14.1.windows.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Michael Niedermayer July 26, 2019, 10:53 p.m. UTC | #2
On Fri, Jul 26, 2019 at 06:45:16PM +0200, Nicolas Gaullier wrote:
> ---
>  libavformat/avformat.h      |  7 +++++++
>  libavformat/mxfdec.c        | 20 +++++++++++++++++++-
>  libavformat/options_table.h |  1 +
>  libavformat/wavdec.c        |  7 ++++++-
>  4 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 6eb329f13f..42bb094d81 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1951,6 +1951,13 @@ typedef struct AVFormatContext {
>       * - decoding: set by user
>       */
>      int skip_estimate_duration_from_pts;
> +
> +    /**
> +     * Probe dolby_e in PCM streams
> +     * - encoding: unused
> +     * - decoding: set by user
> +     */
> +    int dolby_e_probe;
>  } AVFormatContext;
>  
>  #if FF_API_FORMAT_GET_SET
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index bb72fb9841..5b6eb9d756 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -56,6 +56,7 @@
>  #include "avformat.h"
>  #include "internal.h"
>  #include "mxf.h"
> +#include "s337m.h"
>  
>  #define MXF_MAX_CHUNK_SIZE (32 << 20)
>  

> @@ -302,6 +303,8 @@ typedef struct MXFMetadataReadTableEntry {
>      enum MXFMetadataSetType type;
>  } MXFMetadataReadTableEntry;
>  
> +static int mxf_read_packet_init = 0;

non constant static, 
container instance based variables should be in the context


[...]
Tomas Härdin July 30, 2019, 12:02 p.m. UTC | #3
fre 2019-07-26 klockan 18:45 +0200 skrev Nicolas Gaullier:
> @@ -1951,6 +1951,13 @@ typedef struct AVFormatContext {
>       * - decoding: set by user
>       */
>      int skip_estimate_duration_from_pts;
> +
> +    /**
> +     * Probe dolby_e in PCM streams
> +     * - encoding: unused
> +     * - decoding: set by user
> +     */
> +    int dolby_e_probe;

This probably belongs in MXFContext and WAVDemuxContext

>  } AVFormatContext;
>  
>  #if FF_API_FORMAT_GET_SET
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index bb72fb9841..5b6eb9d756 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -56,6 +56,7 @@
>  #include "avformat.h"
>  #include "internal.h"
>  #include "mxf.h"
> +#include "s337m.h"
>  
>  #define MXF_MAX_CHUNK_SIZE (32 << 20)
>  
> @@ -302,6 +303,8 @@ typedef struct MXFMetadataReadTableEntry {
>      enum MXFMetadataSetType type;
>  } MXFMetadataReadTableEntry;
>  
> +static int mxf_read_packet_init = 0;

This definitely belongs in MXFContext

> @@ -3247,6 +3250,19 @@ static int mxf_read_header(AVFormatContext *s)
>      if ((ret = mxf_parse_structural_metadata(mxf)) < 0)
>          goto fail;
>  
> +    if (mxf->fc->dolby_e_probe)
> +    {
> +        int i;
> +        AVPacket *pkt = av_packet_alloc();
> +        av_init_packet(pkt);
> +        mxf_read_packet_init = 1;
> +        for (i = 0; i < mxf->fc->nb_streams; i++)
> +            mxf_read_packet(mxf->fc, pkt);

mxf_read_packet() has a whole bunch of side effects. I'm not so sure
you want to use it here.

> @@ -3539,7 +3555,9 @@ static int mxf_read_packet(AVFormatContext *s,
> AVPacket *pkt)
>                      return ret;
>                  }
>              } else {
> -                ret = av_get_packet(s->pb, pkt, klv.length);
> +                if (mxf_read_packet_init)
> +                    s337m_probe_stream(mxf->fc, &st);
> +                ret = st->codecpar->codec_id == AV_CODEC_ID_DOLBY_E
> ? s337m_read_packet(s, pkt) : av_get_packet(s->pb, pkt, klv.length);

Are you sure there's no UL for this? Did you poke the company who
created the files to have them fix their encoder?

> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
> index 52194f54ef..501c21f220 100644
> --- a/libavformat/wavdec.c
> +++ b/libavformat/wavdec.c
> @@ -41,6 +41,7 @@
>  #include "riff.h"
>  #include "w64.h"
>  #include "spdif.h"
> +#include "s337m.h"
>  
>  typedef struct WAVDemuxContext {
>      const AVClass *class;
> @@ -593,6 +594,8 @@ 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 (s->dolby_e_probe)
> +        s337m_probe_stream(s, &st);

The same goes here - these codecs should have corresponding TwoCCs.
Send angry emails to your vendor.

/Tomas
Carl Eugen Hoyos July 30, 2019, 1:24 p.m. UTC | #4
> Am 30.07.2019 um 14:02 schrieb Tomas Härdin <tjoppen@acc.umu.se>:
> 
> fre 2019-07-26 klockan 18:45 +0200 skrev Nicolas Gaullier:
> 
>> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
>> index 52194f54ef..501c21f220 100644
>> --- a/libavformat/wavdec.c
>> +++ b/libavformat/wavdec.c
>> @@ -41,6 +41,7 @@
>> #include "riff.h"
>> #include "w64.h"
>> #include "spdif.h"
>> +#include "s337m.h"
>> 
>> typedef struct WAVDemuxContext {
>>     const AVClass *class;
>> @@ -593,6 +594,8 @@ 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 (s->dolby_e_probe)
>> +        s337m_probe_stream(s, &st);
> 
> The same goes here - these codecs should have corresponding TwoCCs.

There is definitely no TwoCCs for these kind of files, same as for DTS and AC-3. (I don't know about Dolby E in mxf.)
A probe function is definitely needed, it maybe should be more similar to existing lavfi probe functions though.

Carl Eugen
Tomas Härdin July 30, 2019, 7:04 p.m. UTC | #5
tis 2019-07-30 klockan 15:24 +0200 skrev Carl Eugen Hoyos:
> 
> 
> > Am 30.07.2019 um 14:02 schrieb Tomas Härdin <tjoppen@acc.umu.se>:
> > 
> > fre 2019-07-26 klockan 18:45 +0200 skrev Nicolas Gaullier:
> > 
> > > diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
> > > index 52194f54ef..501c21f220 100644
> > > --- a/libavformat/wavdec.c
> > > +++ b/libavformat/wavdec.c
> > > @@ -41,6 +41,7 @@
> > > #include "riff.h"
> > > #include "w64.h"
> > > #include "spdif.h"
> > > +#include "s337m.h"
> > > 
> > > typedef struct WAVDemuxContext {
> > >     const AVClass *class;
> > > @@ -593,6 +594,8 @@ 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 (s->dolby_e_probe)
> > > +        s337m_probe_stream(s, &st);
> > 
> > The same goes here - these codecs should have corresponding TwoCCs.
> 
> There is definitely no TwoCCs for these kind of files, same as for
> DTS and AC-3. (I don't know about Dolby E in mxf.)

MXF has a UL registered for Dolby-E:
urn:smpte:ul:060e2b34.04010101.04020202.03021c00
We should continue to reject files that don't use it

> A probe function is definitely needed, it maybe should be more
> similar to existing lavfi probe functions though.

It's easy enough to probe this in .wav since it only involved reading a
few bytes from the data chunk, even if a proper TwoCC would be highly
preferable.

Both these situation could be handled by avformat_find_stream_info() if
there's some way to detect that a file is using AES3 in advance, and
delay setting codec_id until the first audio packet has been inspected

/Tomas
Carl Eugen Hoyos July 30, 2019, 7:07 p.m. UTC | #6
Am Di., 30. Juli 2019 um 21:04 Uhr schrieb Tomas Härdin <tjoppen@acc.umu.se>:
>
> tis 2019-07-30 klockan 15:24 +0200 skrev Carl Eugen Hoyos:
> >
> >
> > > Am 30.07.2019 um 14:02 schrieb Tomas Härdin <tjoppen@acc.umu.se>:
> > >
> > > fre 2019-07-26 klockan 18:45 +0200 skrev Nicolas Gaullier:
> > >
> > > > diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
> > > > index 52194f54ef..501c21f220 100644
> > > > --- a/libavformat/wavdec.c
> > > > +++ b/libavformat/wavdec.c
> > > > @@ -41,6 +41,7 @@
> > > > #include "riff.h"
> > > > #include "w64.h"
> > > > #include "spdif.h"
> > > > +#include "s337m.h"
> > > >
> > > > typedef struct WAVDemuxContext {
> > > >     const AVClass *class;
> > > > @@ -593,6 +594,8 @@ 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 (s->dolby_e_probe)
> > > > +        s337m_probe_stream(s, &st);
> > >
> > > The same goes here - these codecs should have corresponding TwoCCs.
> >
> > There is definitely no TwoCCs for these kind of files, same as for
> > DTS and AC-3. (I don't know about Dolby E in mxf.)
>
> MXF has a UL registered for Dolby-E:
> urn:smpte:ul:060e2b34.04010101.04020202.03021c00
> We should continue to reject files that don't use it

Imo, we should auto-detect as much as possible as it was done in
FFmpeg for the last decade.

> > A probe function is definitely needed, it maybe should be more
> > similar to existing lavfi probe functions though.
>
> It's easy enough to probe this in .wav since it only involved reading a
> few bytes from the data chunk, even if a proper TwoCC would be highly
> preferable.

There is no TwoCC.

> Both these situation could be handled by avformat_find_stream_info() if
> there's some way to detect that a file is using AES3 in advance, and
> delay setting codec_id until the first audio packet has been inspected

First step is a demuxer, wav can then use the probe function.

Carl Eugen
Tomas Härdin July 31, 2019, 12:50 p.m. UTC | #7
tis 2019-07-30 klockan 21:07 +0200 skrev Carl Eugen Hoyos:
> Am Di., 30. Juli 2019 um 21:04 Uhr schrieb Tomas Härdin <tjoppen@acc.umu.se>:
> > tis 2019-07-30 klockan 15:24 +0200 skrev Carl Eugen Hoyos:
> > > 
> > > > Am 30.07.2019 um 14:02 schrieb Tomas Härdin <tjoppen@acc.umu.se>:
> > > > 
> > > > fre 2019-07-26 klockan 18:45 +0200 skrev Nicolas Gaullier:
> > > > 
> > > > > diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
> > > > > index 52194f54ef..501c21f220 100644
> > > > > --- a/libavformat/wavdec.c
> > > > > +++ b/libavformat/wavdec.c
> > > > > @@ -41,6 +41,7 @@
> > > > > #include "riff.h"
> > > > > #include "w64.h"
> > > > > #include "spdif.h"
> > > > > +#include "s337m.h"
> > > > > 
> > > > > typedef struct WAVDemuxContext {
> > > > >     const AVClass *class;
> > > > > @@ -593,6 +594,8 @@ 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 (s->dolby_e_probe)
> > > > > +        s337m_probe_stream(s, &st);
> > > > 
> > > > The same goes here - these codecs should have corresponding TwoCCs.
> > > 
> > > There is definitely no TwoCCs for these kind of files, same as for
> > > DTS and AC-3. (I don't know about Dolby E in mxf.)
> > 
> > MXF has a UL registered for Dolby-E:
> > urn:smpte:ul:060e2b34.04010101.04020202.03021c00
> > We should continue to reject files that don't use it
> 
> Imo, we should auto-detect as much as possible as it was done in
> FFmpeg for the last decade.

I actually worded this a bit incorrectly; the file *is* PCM, so
everything is fine as far as mxfdec is concerned. See below

I also notice this UL is already in mxf.c, but added commented out by
Baptiste back in 2006..

> > > A probe function is definitely needed, it maybe should be more
> > > similar to existing lavfi probe functions though.
> > 
> > It's easy enough to probe this in .wav since it only involved reading a
> > few bytes from the data chunk, even if a proper TwoCC would be highly
> > preferable.
> 
> There is no TwoCC.
> 
> > Both these situation could be handled by avformat_find_stream_info() if
> > there's some way to detect that a file is using AES3 in advance, and
> > delay setting codec_id until the first audio packet has been inspected
> 
> First step is a demuxer, wav can then use the probe function.

It demuxes just fine. It's PCM audio in an AES-BWF essence container.

Probing is not what you want, at least not automatic probing. A generic
lavf format option is needed, because Dolby-E can be contained in any
format that supports 16-bit PCM. Which is pretty much all formats.
Nicolas is on the right track, and my gut feeling about per-format
options was wrong.

FFmpeg has no way of knowing the user's intent without options. You'll
have the outer product of at least these cases:

* a file contains PCM data, which does not probe as Dolby-E
* a file contains PCM data, which probes as Dolby-E
* a file contains Dolby-E data, tagged as such (with UL or TwoCC)

* the user has the computer FFmpeg is running on hooked up to another
machine via mini tele or RCA connectors
* the user has the computer FFmpeg is running on hooked up to another
machine via S/PDIF

* the other machine has a Dolby-E decoder, and it should be used
* the other machine has a Dolby-E decoder, and it should not be used
* the other machine has no Dolby-E decoder

So, a general solution is needed. When the user knows the audio is
Dolby-E then they can obviously just override the decoder, if they want
(not always). Another typical case is "if you think the audio is Dolby-
E, automatically decode it so I don't blow my speakers!". An option
could allow avformat_find_stream_info() to wait for the first audio
packet and probe it, and not take what the demuxer says at face value.

/Tomas
Gaullier Nicolas Aug. 1, 2019, 4:34 p.m. UTC | #8
> > > MXF has a UL registered for Dolby-E:

> I also notice this UL is already in mxf.c, but added commented out by Baptiste back in 2006..

There are two ways for signalling dolby-E in MXF : channel status IF using an AES3 descriptor, or the registered UL for Sound Essence Coding.
ARD/ZDF/HDF requires both but it is very specific. The use of the channel status only seems a little bit more common, I have seen it in Harmonic files.
But at the end, what is really highly widespread is to have no signaling at all.

> So, a general solution is needed. When the user knows the audio is Dolby-E then they can obviously just override the decoder, if they want (not always). Another typical case is "if you think the audio is Dolby- E, automatically decode it so I don't blow my speakers!". An option could allow avformat_find_stream_info() to wait for the first audio packet and probe it, and not take what the demuxer says at face value.

Concerning a general solution, the problem is even worse : nowadays MXF files are mostly structured with mono audio tracks (ARD/ZDF, AS-10 in France etc.)...
My idea was to enable the submux only for wav/mxf/potentially quicktime because I don't see use case for other formats; but this require stereo audio.
For a more general solution (MXF with mono audio tracks specifically), I have in mind to build an audio filter : a graph would merge the L/R to build a dolbyE stream that the filter could decode: this is not very handy for users, but I don't know how it could be done otherwise.
I am currently working with an AVOption in MXF&WAV contexts, but if you think avformat_find_stream_info() is a better way, I will take a look... but I am not very confident with my knowledge of ffmpeg for that work, and I will rather certainly concentrate on the general case with the audio filter if this is valid for you ?

Thank you for all your comments
Nicolas
Tomas Härdin Aug. 3, 2019, 1:56 p.m. UTC | #9
tor 2019-08-01 klockan 16:34 +0000 skrev Gaullier Nicolas:
> > > > MXF has a UL registered for Dolby-E:
> > I also notice this UL is already in mxf.c, but added commented out
> > by Baptiste back in 2006..
> There are two ways for signalling dolby-E in MXF : channel status IF
> using an AES3 descriptor, or the registered UL for Sound Essence
> Coding.

Which specs pertain to this?

> ARD/ZDF/HDF requires both but it is very specific. The use of the
> channel status only seems a little bit more common, I have seen it in
> Harmonic files.
> But at the end, what is really highly widespread is to have no
> signaling at all.

Wouldn't be broadcast if the crappiest way wasn't the most widespread

> > So, a general solution is needed. When the user knows the audio is
> > Dolby-E then they can obviously just override the decoder, if they
> > want (not always). Another typical case is "if you think the audio
> > is Dolby- E, automatically decode it so I don't blow my speakers!".
> > An option could allow avformat_find_stream_info() to wait for the
> > first audio packet and probe it, and not take what the demuxer says
> > at face value.
> Concerning a general solution, the problem is even worse : nowadays
> MXF files are mostly structured with mono audio tracks (ARD/ZDF, AS-
> 10 in France etc.)...
> My idea was to enable the submux only for wav/mxf/potentially
> quicktime because I don't see use case for other formats; but this
> require stereo audio.

I wouldn't be surprised if this shows up in GXF and LXF also

> For a more general solution (MXF with mono audio tracks
> specifically), I have in mind to build an audio filter : a graph
> would merge the L/R to build a dolbyE stream that the filter could
> decode: this is not very handy for users, but I don't know how it
> could be done otherwise.

This would suggest that a filter based approach would be best. It
sounds like the user will have to apply quite a bit of business logic
either way, except maybe for files that signal Dolby-E properly

> I am currently working with an AVOption in MXF&WAV contexts, but if
> you think avformat_find_stream_info() is a better way, I will take a
> look... but I am not very confident with my knowledge of ffmpeg for
> that work, and I will rather certainly concentrate on the general
> case with the audio filter if this is valid for you ?

It's been a while since I poked at lavf internals, so I'm not quite
sure what would be the best approach. Maybe Baptiste has some ideas?

Feel free to poke me on IRC about this, since it seems a bit of
discussion is needed (thardin in #ffmpeg-devel on freenode)

/Tomas
diff mbox

Patch

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 6eb329f13f..42bb094d81 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1951,6 +1951,13 @@  typedef struct AVFormatContext {
      * - decoding: set by user
      */
     int skip_estimate_duration_from_pts;
+
+    /**
+     * Probe dolby_e in PCM streams
+     * - encoding: unused
+     * - decoding: set by user
+     */
+    int dolby_e_probe;
 } AVFormatContext;
 
 #if FF_API_FORMAT_GET_SET
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index bb72fb9841..5b6eb9d756 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -56,6 +56,7 @@ 
 #include "avformat.h"
 #include "internal.h"
 #include "mxf.h"
+#include "s337m.h"
 
 #define MXF_MAX_CHUNK_SIZE (32 << 20)
 
@@ -302,6 +303,8 @@  typedef struct MXFMetadataReadTableEntry {
     enum MXFMetadataSetType type;
 } MXFMetadataReadTableEntry;
 
+static int mxf_read_packet_init = 0;
+static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt);
 static int mxf_read_close(AVFormatContext *s);
 
 /* partial keys to match */
@@ -3247,6 +3250,19 @@  static int mxf_read_header(AVFormatContext *s)
     if ((ret = mxf_parse_structural_metadata(mxf)) < 0)
         goto fail;
 
+    if (mxf->fc->dolby_e_probe)
+    {
+        int i;
+        AVPacket *pkt = av_packet_alloc();
+        av_init_packet(pkt);
+        mxf_read_packet_init = 1;
+        for (i = 0; i < mxf->fc->nb_streams; i++)
+            mxf_read_packet(mxf->fc, pkt);
+        mxf_read_packet_init = 0;
+        av_freep(pkt);
+        avio_seek(s->pb, essence_offset, SEEK_SET);
+    }
+
     for (int i = 0; i < s->nb_streams; i++)
         mxf_handle_missing_index_segment(mxf, s->streams[i]);
 
@@ -3539,7 +3555,9 @@  static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt)
                     return ret;
                 }
             } else {
-                ret = av_get_packet(s->pb, pkt, klv.length);
+                if (mxf_read_packet_init)
+                    s337m_probe_stream(mxf->fc, &st);
+                ret = st->codecpar->codec_id == AV_CODEC_ID_DOLBY_E ? s337m_read_packet(s, pkt) : av_get_packet(s->pb, pkt, klv.length);
                 if (ret < 0) {
                     mxf->current_klv_data = (KLVPacket){{0}};
                     return ret;
diff --git a/libavformat/options_table.h b/libavformat/options_table.h
index f2f077b34f..7f3c22d6bb 100644
--- a/libavformat/options_table.h
+++ b/libavformat/options_table.h
@@ -111,6 +111,7 @@  static const AVOption avformat_options[] = {
 {"protocol_blacklist", "List of protocols that are not allowed to be used", OFFSET(protocol_blacklist), AV_OPT_TYPE_STRING, { .str = NULL },  CHAR_MIN, CHAR_MAX, D },
 {"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = 1000 }, 0, INT_MAX, D },
 {"skip_estimate_duration_from_pts", "skip duration calculation in estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D},
+{"dolbyeprobe", "probe dolby_e in PCM streams", OFFSET(dolby_e_probe), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, D},
 {NULL},
 };
 
diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
index 52194f54ef..501c21f220 100644
--- a/libavformat/wavdec.c
+++ b/libavformat/wavdec.c
@@ -41,6 +41,7 @@ 
 #include "riff.h"
 #include "w64.h"
 #include "spdif.h"
+#include "s337m.h"
 
 typedef struct WAVDemuxContext {
     const AVClass *class;
@@ -593,6 +594,8 @@  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 (s->dolby_e_probe)
+        s337m_probe_stream(s, &st);
 
     ff_metadata_conv_ctx(s, NULL, wav_metadata_conv);
     ff_metadata_conv_ctx(s, NULL, ff_riff_info_conv);
@@ -708,7 +711,7 @@  smv_out:
         size = (size / st->codecpar->block_align) * st->codecpar->block_align;
     }
     size = FFMIN(size, left);
-    ret  = av_get_packet(s->pb, pkt, size);
+    ret  = st->codecpar->codec_id == AV_CODEC_ID_DOLBY_E ? s337m_read_packet(s, pkt) : av_get_packet(s->pb, pkt, size);
     if (ret < 0)
         return ret;
     pkt->stream_index = 0;
@@ -895,6 +898,8 @@  static int w64_read_header(AVFormatContext *s)
     st->need_parsing = AVSTREAM_PARSE_FULL_RAW;
 
     avio_seek(pb, data_ofs, SEEK_SET);
+    if (s->dolby_e_probe)
+        s337m_probe_stream(s, &st);
 
     set_spdif(s, wav);