[FFmpeg-devel] [PATCH] lavf/movenc: Write 'dby1' minor brand if Dolby content is being muxed to MP4

[Jan Ekström]

On Tue, Sep 28, 2021 at 5:31 PM Derek Buitenhuis
<derek.buitenhuis at gmail.com> wrote:
>
> This is as per:
>    * mp4ra: http://mp4ra.org/#/brands
>    * Dolby Vision muxing spec (which is public):
>        https://professional.dolby.com/siteassets/content-creation/dolby-vision-for-content-creators/dolby_vision_bitstreams_within_the_iso_base_media_file_format_dec2017.pdf
>
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis at gmail.com>
> ---
> The sole FATE change is just the brand being written for EAC-3.

I do dislike it how outside of the DoVi pdf they don't really seem to
specify 'dby1', but the mp4 registration authority's description goes
all the way back to January 2017 with this identifier
(https://github.com/mp4ra/mp4ra.github.io/blob/a27f402652b57cea190a33f6955a843869fdd457/filetype.html).

f.ex. none of the following seem to mention the 'dby1' brand:

TrueHD/MLP in MP4
https://developer.dolby.com/globalassets/technology/dolby-truehd/dolby-truehd-mlp-bitstreams-within-the-iso-base-media-file-format.pdf

(E-)AC-3 in MP4
https://www.etsi.org/deliver/etsi_ts/102300_102399/102366/01.04.01_60/ts_102366v010401p.pdf
(E-)AC-3 with Object Based Audio in MP4
https://www.etsi.org/deliver/etsi_ts/103400_103499/103420/01.02.01_60/ts_103420v010201p.pdf

AC-4 Part 1 in MP4
https://www.etsi.org/deliver/etsi_ts/103100_103199/10319001/01.03.01_60/ts_10319001v010301p.pdf
AC-4 Part 2 in MP4
https://www.etsi.org/deliver/etsi_ts/103100_103199/10319002/01.02.01_60/ts_10319002v010201p.pdf

> ---
>  libavformat/movenc.c         | 9 ++++++++-
>  tests/ref/fate/copy-trac3074 | 4 ++--
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 7650ac5ed3..fe3405d271 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -4991,11 +4991,13 @@ static int mov_write_ftyp_tag(AVIOContext *pb, AVFormatContext *s)
>  {
>      MOVMuxContext *mov = s->priv_data;
>      int64_t pos = avio_tell(pb);
> -    int has_h264 = 0, has_av1 = 0, has_video = 0;
> +    int has_h264 = 0, has_av1 = 0, has_video = 0, has_dolby = 0;
>      int i;
>
>      for (i = 0; i < s->nb_streams; i++) {
>          AVStream *st = s->streams[i];
> +        AVDOVIDecoderConfigurationRecord *dovi = (AVDOVIDecoderConfigurationRecord *)
> +                                                     av_stream_get_side_data(st, AV_PKT_DATA_DOVI_CONF, NULL);

Maybe something a la the following to keep the line length shorter?

+        AVDOVIDecoderConfigurationRecord *dovi =
+            (AVDOVIDecoderConfigurationRecord *)
+            av_stream_get_side_data(st, AV_PKT_DATA_DOVI_CONF, NULL);
+

>          if (is_cover_image(st))
>              continue;
>          if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO)
> @@ -5004,6 +5006,9 @@ static int mov_write_ftyp_tag(AVIOContext *pb, AVFormatContext *s)
>              has_h264 = 1;
>          if (st->codecpar->codec_id == AV_CODEC_ID_AV1)
>              has_av1 = 1;
> +        if (dovi || st->codecpar->codec_id == AV_CODEC_ID_AC3 ||
> +            st->codecpar->codec_id == AV_CODEC_ID_EAC3 || st->codecpar->codec_id == AV_CODEC_ID_TRUEHD)
> +            has_dolby = 1;

Maybe something a la the following to keep the line length shorter
(and also the codec_id checks aligned)?
+        if (dovi ||
+            st->codecpar->codec_id == AV_CODEC_ID_AC3 ||
+            st->codecpar->codec_id == AV_CODEC_ID_EAC3 ||
+            st->codecpar->codec_id == AV_CODEC_ID_TRUEHD)

(I think the next step around this code would almost be a switch/case
thing since we've got multiple of them now :D)

Otherwise LGTM.

Jan