[FFmpeg-devel] [PATCH] lavf/mux: allow unofficial extension by default for spatial data in mp4

[Linjie Fu]

On Fri, Aug 28, 2020 at 11:18 AM Linjie Fu <linjie.justin.fu at gmail.com> wrote:
>
> Hi,
>
> On Fri, Aug 28, 2020 at 8:46 AM Andreas Rheinhardt
> <andreas.rheinhardt at gmail.com> wrote:
> >
> > Linjie Fu:
> > > Mp4 unofficial extension allows Stereo3D and Spherical Mapping data
> > > in header if users explicitly set "-strict unofficial" or values less
> > > than "-1".
> > >
> > > Currently side_data like stereo3D and spherical data in mp4 would be
> > > dropped by default in a transcoding procedure even if user uses streamcopy
> > > to get the same bitstreams. Spatial data missing in containers seems to
> > > cause troubles for the players like VLC while detecting the projection type
> > > for 360 video.
> > >
> > > Set the default value of "strict" to "unofficial" for mp4.
> > >
> > > Signed-off-by: Linjie Fu <linjie.justin.fu at gmail.com>
> > > ---
> > > I would prefer to add the default value for format-specific options in something
> > > like AVCodecDefault, however didn't find one.
> > >  libavformat/mux.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/libavformat/mux.c b/libavformat/mux.c
> > > index 44d5e5d1c0..ffb9109a6f 100644
> > > --- a/libavformat/mux.c
> > > +++ b/libavformat/mux.c
> > > @@ -237,6 +237,11 @@ static int init_muxer(AVFormatContext *s, AVDictionary **options)
> > >      const AVCodecDescriptor *desc;
> > >      AVDictionaryEntry *e;
> > >
> > > +    if (s->oformat) {
> > > +        if (!strcmp("mp4", s->oformat->name))
> > > +            s->strict_std_compliance = FF_COMPLIANCE_UNOFFICIAL;
> > > +    }
> > > +
> > >      if (options)
> > >          av_dict_copy(&tmp, *options, 0);
> > >
> > 1. s->oformat must already be set at this point.
> > 2. You seem to believe that the only way for the user to set
> > strict_std_compliance etc. is by providing via the a dict provided to
> > avformat_init_output/write_header(). This is wrong. The user can also
> > set it before that. Your patch would break this.
>
> It's like a priority order issue of:
> 1. user specified somehow in AVFormatContext;
> 2. user explicitly sets through options;
> 3. default value for specific container.
>
> The logic for profile choosing in libopenh264enc seems to be good if we we had
> a default value of something like "unknown" instead of "normal", since we can't
> tell whether the "normal" is derived by user's choice or just a default value:
> https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/libopenh264enc.c#L198
>
> > Obviously you have no way to distinguish whether the user explicitly set
> > the option to FF_COMPLIANCE_NORMAL (and even less way to know whether
> > the user intentionally doesn't want unofficial extensions and therefore
> > opted to not touch strict_std_compliance).
>
> If I understood it correctly, user explicitly setting "strict" to
> "normal" is able to be distinguished.
> av_opt_set_dict() is able to replace the default "unofficial" with the
> user specified "normal":
> https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/mux.c#L243
>
> > 3. One could print a warning in case the such unofficial extensions are
> > dropped.
> Sure, a warning should be added.
>
> > 3. How "unofficial" is this extension anyway? After all, the default
> > mode intentionally differs from FF_COMPLIANCE_STRICT.
>
> Defined as unofficial in 86dee47e397fe6bb0907adae8d4a54138a947646.
>
> Seems to be defined in:
> https://github.com/google/spatial-media/blob/master/docs/spherical-video-v2-rfc.md
>
> which is also cited/referenced by youtube hellp with details for
> adding metadata by ffmpeg:
> https://support.google.com/youtube/answer/7278886?hl=en
>

To make this easier to be reproduced/verified, attached the clips:
https://drive.google.com/file/d/1tU1HuPvCzOyYY3UBp2Eh7Xi8NWldczxD/view?usp=sharing

$ ffmpeg -i Professional+4K+360°+live+streaming+with+Insta360+Pro+and+Yi+360+VR.mp4
-c:v copy -an -y out.mp4
$ ffmpeg -i out.mp4
Then check the side data information.