[FFmpeg-devel] [PATCH v2 1/2] lavf/isom: support for demuxing MPEG-H 3D Audio in MP4

Tsuchiya, Yuki (SHES) Yuki.Tsuchiya at sony.com
Thu Oct 31 06:44:04 EET 2019


Hi,

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Paul
> B Mahol
> Sent: Wednesday, October 30, 2019 6:52 PM
> To: FFmpeg development discussions and patches
> <ffmpeg-devel at ffmpeg.org>
> Cc: Tsuchiya, Yuki (SHES) <Yuki.Tsuchiya at sony.com>
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] lavf/isom: support for demuxing
> MPEG-H 3D Audio in MP4
> > +- Support for demuxing MPEG-H 3D Audio in mp4

> Unacceptable change.
Could you point out the reason why is this patch not acceptable?

> > +    AV_CODEC_ID_MPEGH_3D_AUDIO,
> 
> Does this really need _AUDIO suffix?
There is no short name or other name regarding MPEG-H 3D Audio codec as of this moment.
If AV_CODEC_ID_MPEGH_3D_AUDIO is too redundant, just "AV_CODEC_ID_MPEGH_3D" will also work.

> >  #define LIBAVCODEC_VERSION_MINOR  60
> > -#define LIBAVCODEC_VERSION_MICRO 100
> > +#define LIBAVCODEC_VERSION_MICRO 101
> 
> Not good.
Should change minor version? Or not necessary?

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Paul
> B Mahol
> Sent: Wednesday, October 30, 2019 6:52 PM
> To: FFmpeg development discussions and patches
> <ffmpeg-devel at ffmpeg.org>
> Cc: Tsuchiya, Yuki (SHES) <Yuki.Tsuchiya at sony.com>
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] lavf/isom: support for demuxing
> MPEG-H 3D Audio in MP4
> 
> On 10/30/19, Yuki.Tsuchiya <Yuki.Tsuchiya at sony.com> wrote:
> > Implemented according to the specification at
> > https://www.iso.org/standard/69561.html
> > The 'mhm1' sample entry is registered with MP4RA, which is defined as
> > MHAS encapsulated single stream MPEG-H 3D Audio.
> > 'MHAS' stands for MPEG-H audio stream, which contains encoded audio
> > data and corresponds metadata for decoding.
> > This patch enables extracting the MHAS bitstream from MP4.
> >
> > Signed-off-by: Yuki.Tsuchiya <Yuki.Tsuchiya at sony.com>
> > ---
> >  Changelog               | 1 +
> >  libavcodec/avcodec.h    | 1 +
> >  libavcodec/codec_desc.c | 7 +++++++
> >  libavcodec/version.h    | 2 +-
> >  libavformat/isom.c      | 1 +
> >  libavformat/movenc.c    | 6 ++++--
> >  libavformat/utils.c     | 3 ++-
> >  7 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/Changelog b/Changelog
> > index 7c971fc..afb3bff 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -20,6 +20,7 @@ version <next>:
> >  - maskedmin and maskedmax filters
> >  - VDPAU VP9 hwaccel
> >  - median filter
> > +- Support for demuxing MPEG-H 3D Audio in mp4
> 
> Unacceptable change.
> 
> >
> >
> >  version 4.2:
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index
> > bcb931f..8c1a85d 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -654,6 +654,7 @@ enum AVCodecID {
> >      AV_CODEC_ID_ATRAC9,
> >      AV_CODEC_ID_HCOM,
> >      AV_CODEC_ID_ACELP_KELVIN,
> > +    AV_CODEC_ID_MPEGH_3D_AUDIO,
> 
> Does this really need _AUDIO suffix?
> 
> >
> >      /* subtitle codecs */
> >      AV_CODEC_ID_FIRST_SUBTITLE = 0x17000,          ///< A dummy
> ID pointing
> > at the start of subtitle codecs.
> > diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c index
> > 0602ecb..a970fae 100644
> > --- a/libavcodec/codec_desc.c
> > +++ b/libavcodec/codec_desc.c
> > @@ -2998,6 +2998,13 @@ static const AVCodecDescriptor
> > codec_descriptors[] = {
> >          .long_name = NULL_IF_CONFIG_SMALL("Sipro ACELP.KELVIN"),
> >          .props     = AV_CODEC_PROP_LOSSY,
> >      },
> > +    {
> > +        .id        = AV_CODEC_ID_MPEGH_3D_AUDIO,
> > +        .type      = AVMEDIA_TYPE_AUDIO,
> > +        .name      = "mpegh_3d_audio",
> 
> Same here.
> 
> > +        .long_name = NULL_IF_CONFIG_SMALL("MPEG-H 3D Audio"),
> > +        .props     = AV_CODEC_PROP_LOSSY,
> > +    },
> >
> >      /* subtitle codecs */
> >      {
> > diff --git a/libavcodec/version.h b/libavcodec/version.h index
> > 27c126e..64e2365 100644
> > --- a/libavcodec/version.h
> > +++ b/libavcodec/version.h
> > @@ -29,7 +29,7 @@
> >
> >  #define LIBAVCODEC_VERSION_MAJOR  58
> >  #define LIBAVCODEC_VERSION_MINOR  60
> > -#define LIBAVCODEC_VERSION_MICRO 100
> > +#define LIBAVCODEC_VERSION_MICRO 101
> 
> Not good.
> 
> >
> >  #define LIBAVCODEC_VERSION_INT
> AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> >
> > LIBAVCODEC_VERSION_MINOR, \ diff --git a/libavformat/isom.c
> > b/libavformat/isom.c index edd0d81..824e811 100644
> > --- a/libavformat/isom.c
> > +++ b/libavformat/isom.c
> > @@ -371,6 +371,7 @@ const AVCodecTag ff_codec_movaudio_tags[] = {
> >      { AV_CODEC_ID_FLAC,            MKTAG('f', 'L', 'a', 'C') }, /*
> > nonstandard */
> >      { AV_CODEC_ID_TRUEHD,          MKTAG('m', 'l', 'p', 'a') }, /*
> > mp4ra.org */
> >      { AV_CODEC_ID_OPUS,            MKTAG('O', 'p', 'u', 's') }, /*
> > mp4ra.org */
> > +    { AV_CODEC_ID_MPEGH_3D_AUDIO,  MKTAG('m', 'h', 'm', '1') }, /*
> > + MPEG-H
> > 3D Audio bitstream */
> >      { AV_CODEC_ID_NONE, 0 },
> >  };
> >
> > diff --git a/libavformat/movenc.c b/libavformat/movenc.c index
> > 715bec1..ff234d9 100644
> > --- a/libavformat/movenc.c
> > +++ b/libavformat/movenc.c
> > @@ -2411,7 +2411,7 @@ static int
> > mov_preroll_write_stbl_atoms(AVIOContext
> > *pb, MOVTrack *track)
> >      if (!sgpd_entries)
> >          return AVERROR(ENOMEM);
> >
> > -    av_assert0(track->par->codec_id == AV_CODEC_ID_OPUS ||
> > track->par->codec_id == AV_CODEC_ID_AAC);
> > +    av_assert0(track->par->codec_id == AV_CODEC_ID_OPUS ||
> > track->par->codec_id == AV_CODEC_ID_AAC || track->par->codec_id
> ==
> > AV_CODEC_ID_MPEGH_3D_AUDIO);
> >
> >      if (track->par->codec_id == AV_CODEC_ID_OPUS) {
> >          for (i = 0; i < track->entry; i++) { @@ -2493,6 +2493,7 @@
> > static int mov_write_stbl_tag(AVFormatContext *s, AVIOContext *pb,
> > MOVMuxContext
> >      mov_write_stts_tag(pb, track);
> >      if ((track->par->codec_type == AVMEDIA_TYPE_VIDEO ||
> >           track->par->codec_id == AV_CODEC_ID_TRUEHD ||
> > +         track->par->codec_id == AV_CODEC_ID_MPEGH_3D_AUDIO ||
> >           track->par->codec_tag == MKTAG('r','t','p',' ')) &&
> >          track->has_keyframes && track->has_keyframes < track->entry)
> >          mov_write_stss_tag(pb, track, MOV_SYNC_SAMPLE); @@
> -2512,7
> > +2513,7 @@ static int mov_write_stbl_tag(AVFormatContext *s,
> > AVIOContext *pb, MOVMuxContext
> >      if (track->cenc.aes_ctr) {
> >          ff_mov_cenc_write_stbl_atoms(&track->cenc, pb);
> >      }
> > -    if (track->par->codec_id == AV_CODEC_ID_OPUS ||
> track->par->codec_id ==
> > AV_CODEC_ID_AAC) {
> > +    if (track->par->codec_id == AV_CODEC_ID_OPUS ||
> > + track->par->codec_id ==
> > AV_CODEC_ID_AAC || track->par->codec_id ==
> AV_CODEC_ID_MPEGH_3D_AUDIO) {
> >          mov_preroll_write_stbl_atoms(pb, track);
> >      }
> >      return update_size(pb, pos);
> > @@ -6877,6 +6878,7 @@ const AVCodecTag codec_mp4_tags[] = {
> >      { AV_CODEC_ID_DVD_SUBTITLE, MKTAG('m', 'p', '4', 's') },
> >      { AV_CODEC_ID_MOV_TEXT    , MKTAG('t', 'x', '3', 'g') },
> >      { AV_CODEC_ID_BIN_DATA    , MKTAG('g', 'p', 'm', 'd') },
> > +    { AV_CODEC_ID_MPEGH_3D_AUDIO, MKTAG('m', 'h', 'm', '1') },
> >      { AV_CODEC_ID_NONE        ,    0 },
> >  };
> >
> > diff --git a/libavformat/utils.c b/libavformat/utils.c index
> > cfb6d03..d271251 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -1021,7 +1021,8 @@ static int is_intra_only(enum AVCodecID id)
> >      const AVCodecDescriptor *d = avcodec_descriptor_get(id);
> >      if (!d)
> >          return 0;
> > -    if (d->type == AVMEDIA_TYPE_VIDEO && !(d->props &
> > AV_CODEC_PROP_INTRA_ONLY))
> > +    if ((d->type == AVMEDIA_TYPE_VIDEO && !(d->props &
> > AV_CODEC_PROP_INTRA_ONLY)) ||
> > +        id == AV_CODEC_ID_MPEGH_3D_AUDIO)
> >          return 0;
> >      return 1;
> >  }
> > --
> > 2.7.4
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request at ffmpeg.org
> with subject "unsubscribe".


More information about the ffmpeg-devel mailing list