Re: [FFmpeg-devel] [PATCH v10 2/5] avformat/matroska{dec, enc}: Parse BlockAdditionMapping elements

Re: [FFmpeg-devel] [PATCH v10 2/5] avformat/matroska{dec, enc}: Parse BlockAdditionMapping elements

[Andreas Rheinhardt]

quietvoid:
> Adds handling of dvcC/dvvC block addition mappings.
> 
> The parsing creates AVDOVIDecoderConfigurationRecord side data.
> The configuration block is written when muxing into Matroska,
> if DOVI side data is present for the track.
> 
> Most of the Matroska element parsing is based on Plex's FFmpeg source code.
> 
> Signed-off-by: quietvoid <tcChlisop0@gmail.com>
> ---
>  libavformat/Makefile      |  4 +--
>  libavformat/matroska.h    |  9 ++++++
>  libavformat/matroskadec.c | 58 +++++++++++++++++++++++++++++++++++++--
>  libavformat/matroskaenc.c | 37 +++++++++++++++++++++++++
>  4 files changed, 104 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 2b5caf9d33..bd12562a24 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -314,11 +314,11 @@ OBJS-$(CONFIG_M4V_MUXER)                 += rawenc.o
>  OBJS-$(CONFIG_MATROSKA_DEMUXER)          += matroskadec.o matroska.o  \
>                                              flac_picture.o isom_tags.o rmsipr.o \
>                                              oggparsevorbis.o vorbiscomment.o \
> -                                            qtpalette.o replaygain.o
> +                                            qtpalette.o replaygain.o dovi_isom.o
>  OBJS-$(CONFIG_MATROSKA_MUXER)            += matroskaenc.o matroska.o \
>                                              av1.o avc.o hevc.o isom_tags.o \
>                                              flacenc_header.o avlanguage.o \
> -                                            vorbiscomment.o wv.o
> +                                            vorbiscomment.o wv.o dovi_isom.o
>  OBJS-$(CONFIG_MCA_DEMUXER)               += mca.o
>  OBJS-$(CONFIG_MCC_DEMUXER)               += mccdec.o subtitles.o
>  OBJS-$(CONFIG_MD5_MUXER)                 += hashenc.o
> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
> index 2d04a6838b..16491aae22 100644
> --- a/libavformat/matroska.h
> +++ b/libavformat/matroska.h
> @@ -111,6 +111,7 @@
>  #define MATROSKA_ID_TRACKCONTENTENCODING 0x6240
>  #define MATROSKA_ID_TRACKTIMECODESCALE 0x23314F
>  #define MATROSKA_ID_TRACKMAXBLKADDID 0x55EE
> +#define MATROSKA_ID_TRACKBLKADDMAPPING 0x41E4
>  
>  /* IDs in the trackvideo master */
>  #define MATROSKA_ID_VIDEOFRAMERATE 0x2383E3
> @@ -189,6 +190,12 @@
>  #define MATROSKA_ID_ENCODINGSIGKEYID 0x47E4
>  #define MATROSKA_ID_ENCODINGSIGNATURE 0x47E3
>  
> +/* IDs in the block addition mapping master */
> +#define MATROSKA_ID_BLKADDIDVALUE 0x41F0
> +#define MATROSKA_ID_BLKADDIDNAME 0x41A4
> +#define MATROSKA_ID_BLKADDIDTYPE 0x41E7
> +#define MATROSKA_ID_BLKADDIDEXTRADATA 0x41ED
> +
>  /* ID in the cues master */
>  #define MATROSKA_ID_POINTENTRY 0xBB
>  
> @@ -385,4 +392,6 @@ extern const char * const ff_matroska_video_stereo_plane[MATROSKA_VIDEO_STEREO_P
>  
>  int ff_mkv_stereo3d_conv(AVStream *st, MatroskaVideoStereoModeType stereo_mode);
>  
> +#define DVCC_DVVC_BLOCK_TYPE_NAME "Dolby Vision configuration"
> +
>  #endif /* AVFORMAT_MATROSKA_H */
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index a4bbbe954e..6ce553205d 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -53,6 +53,7 @@
>  
>  #include "avformat.h"
>  #include "avio_internal.h"
> +#include "dovi_isom.h"
>  #include "internal.h"
>  #include "isom.h"
>  #include "matroska.h"
> @@ -239,6 +240,13 @@ typedef struct MatroskaTrackOperation {
>      EbmlList combine_planes;
>  } MatroskaTrackOperation;
>  
> +typedef struct MatroskaBlockAdditionMapping {
> +    uint64_t value;
> +    char *name;
> +    uint64_t type;
> +    EbmlBin extradata;
> +} MatroskaBlockAdditionMapping;
> +
>  typedef struct MatroskaTrack {
>      uint64_t num;
>      uint64_t uid;
> @@ -269,6 +277,7 @@ typedef struct MatroskaTrack {
>      int ms_compat;
>      int needs_decoding;
>      uint64_t max_block_additional_id;
> +    EbmlList block_addition_mappings;
>  
>      uint32_t palette[AVPALETTE_COUNT];
>      int has_palette;
> @@ -419,8 +428,8 @@ typedef struct MatroskaDemuxContext {
>  // incomplete type (6.7.2 in C90, 6.9.2 in C99).
>  // Removing the sizes breaks MSVC.
>  static EbmlSyntax ebml_syntax[3], matroska_segment[9], matroska_track_video_color[15], matroska_track_video[19],
> -                  matroska_track[32], matroska_track_encoding[6], matroska_track_encodings[2],
> -                  matroska_track_combine_planes[2], matroska_track_operation[2], matroska_tracks[2],
> +                  matroska_track[33], matroska_track_encoding[6], matroska_track_encodings[2],
> +                  matroska_track_combine_planes[2], matroska_track_operation[2], matroska_block_addition_mapping[5], matroska_tracks[2],
>                    matroska_attachments[2], matroska_chapter_entry[9], matroska_chapter[6], matroska_chapters[2],
>                    matroska_index_entry[3], matroska_index[2], matroska_tag[3], matroska_tags[2], matroska_seekhead[2],
>                    matroska_blockadditions[2], matroska_blockgroup[8], matroska_cluster_parsing[8];
> @@ -570,6 +579,14 @@ static EbmlSyntax matroska_track_operation[] = {
>      CHILD_OF(matroska_track)
>  };
>  
> +static EbmlSyntax matroska_block_addition_mapping[] = {
> +    { MATROSKA_ID_BLKADDIDVALUE,      EBML_UINT, 0, 0, offsetof(MatroskaBlockAdditionMapping, value) },
> +    { MATROSKA_ID_BLKADDIDNAME,       EBML_STR,  0, 0, offsetof(MatroskaBlockAdditionMapping, name) },
> +    { MATROSKA_ID_BLKADDIDTYPE,       EBML_UINT, 0, 0, offsetof(MatroskaBlockAdditionMapping, type) },
> +    { MATROSKA_ID_BLKADDIDEXTRADATA,  EBML_BIN,  0, 0, offsetof(MatroskaBlockAdditionMapping, extradata) },
> +    CHILD_OF(matroska_track)
> +};
> +
>  static EbmlSyntax matroska_track[] = {
>      { MATROSKA_ID_TRACKNUMBER,           EBML_UINT,  0, 0, offsetof(MatroskaTrack, num) },
>      { MATROSKA_ID_TRACKNAME,             EBML_UTF8,  0, 0, offsetof(MatroskaTrack, name) },
> @@ -593,6 +610,7 @@ static EbmlSyntax matroska_track[] = {
>      { MATROSKA_ID_TRACKOPERATION,        EBML_NEST,  0, 0, offsetof(MatroskaTrack, operation),    { .n = matroska_track_operation } },
>      { MATROSKA_ID_TRACKCONTENTENCODINGS, EBML_NEST,  0, 0, 0,                                     { .n = matroska_track_encodings } },
>      { MATROSKA_ID_TRACKMAXBLKADDID,      EBML_UINT,  0, 0, offsetof(MatroskaTrack, max_block_additional_id), { .u = 0 } },
> +    { MATROSKA_ID_TRACKBLKADDMAPPING,    EBML_NEST,  0, sizeof(MatroskaBlockAdditionMapping), offsetof(MatroskaTrack, block_addition_mappings), { .n = matroska_block_addition_mapping } },
>      { MATROSKA_ID_SEEKPREROLL,           EBML_UINT,  0, 0, offsetof(MatroskaTrack, seek_preroll), { .u = 0 } },
>      { MATROSKA_ID_TRACKFLAGENABLED,      EBML_NONE },
>      { MATROSKA_ID_TRACKFLAGLACING,       EBML_NONE },
> @@ -2311,6 +2329,38 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track,
>      return 0;
>  }
>  
> +static int mkv_parse_dvcc_dvvc(AVFormatContext *s, AVStream *st, const MatroskaTrack *track,
> +                               EbmlBin *bin)
> +{
> +    return ff_isom_parse_dvcc_dvvc(s, st, bin->data, bin->size);
> +}
> +
> +static int mkv_parse_block_addition_mappings(AVFormatContext *s, AVStream *st, const MatroskaTrack *track)
> +{
> +    const EbmlList *mappings_list = &track->block_addition_mappings;
> +    MatroskaBlockAdditionMapping *mappings = mappings_list->elem;
> +    int ret;
> +
> +    for (int i = 0; i < mappings_list->nb_elem; i++) {
> +        MatroskaBlockAdditionMapping *mapping = &mappings[i];
> +
> +        switch (mapping->type) {
> +        case MKBETAG('d','v','c','C'):
> +        case MKBETAG('d','v','v','C'):
> +            if ((ret = mkv_parse_dvcc_dvvc(s, st, track, &mapping->extradata)) < 0)
> +                return ret;
> +
> +            break;
> +        default:
> +            av_log(s, AV_LOG_DEBUG,
> +                   "Unknown block additional mapping type 0x%"PRIx64", value %"PRIu64", name \"%s\"\n",
> +                   mapping->type, mapping->value, mapping->name ? mapping->name : "");
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  static int get_qt_codec(MatroskaTrack *track, uint32_t *fourcc, enum AVCodecID *codec_id)
>  {
>      const AVCodecTag *codec_tags;
> @@ -2898,6 +2948,10 @@ static int matroska_parse_tracks(AVFormatContext *s)
>              if (track->flag_textdescriptions)
>                  st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
>          }
> +
> +        ret = mkv_parse_block_addition_mappings(s, st, track);
> +        if (ret < 0)
> +            return ret;
>      }
>  
>      return 0;
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 8c4cf4024a..31b7d7e171 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -27,6 +27,7 @@
>  #include "avformat.h"
>  #include "avio_internal.h"
>  #include "avlanguage.h"
> +#include "dovi_isom.h"
>  #include "flacenc.h"
>  #include "internal.h"
>  #include "isom.h"
> @@ -1120,6 +1121,37 @@ static int mkv_write_stereo_mode(AVFormatContext *s, AVIOContext *pb,
>      return 0;
>  }
>  
> +static void mkv_write_dovi(AVFormatContext *s, AVIOContext *pb, AVStream *st)
> +{
> +    AVDOVIDecoderConfigurationRecord *dovi = (AVDOVIDecoderConfigurationRecord *)
> +                                             av_stream_get_side_data(st, AV_PKT_DATA_DOVI_CONF, NULL);
> +
> +    if (dovi) {
> +        ebml_master mapping;
> +        uint8_t buf[ISOM_DVCC_DVVC_SIZE];
> +        uint32_t type;
> +
> +        uint64_t expected_size = (2 + 1 + (sizeof(DVCC_DVVC_BLOCK_TYPE_NAME) - 1))
> +                                + (2 + 1 + 4) + (2 + 1 + ISOM_DVCC_DVVC_SIZE);
> +
> +        if (dovi->dv_profile > 7) {
> +            type = MKBETAG('d', 'v', 'v', 'C');
> +        } else {
> +            type = MKBETAG('d', 'v', 'c', 'C');
> +        }

This here is wrong, as dv_profiles > 10 need dvwC, which is currently
not supported in Matroska. The above check should be adapted to
"dovi && dovi->dv_profile <= 10".

> +
> +        ff_isom_put_dvcc_dvvc(s, buf, dovi);
> +
> +        mapping = start_ebml_master(pb, MATROSKA_ID_TRACKBLKADDMAPPING, expected_size);
> +
> +        put_ebml_string(pb, MATROSKA_ID_BLKADDIDNAME, DVCC_DVVC_BLOCK_TYPE_NAME);
> +        put_ebml_uint(pb, MATROSKA_ID_BLKADDIDTYPE, type);
> +        put_ebml_binary(pb, MATROSKA_ID_BLKADDIDEXTRADATA, buf, sizeof(buf));
> +
> +        end_ebml_master(pb, mapping);
> +    }
> +}
> +
>  static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>                             AVStream *st, mkv_track *track, AVIOContext *pb,
>                             int is_default)
> @@ -1319,6 +1351,11 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>          mkv_write_video_projection(s, pb, st);
>  
>          end_ebml_master(pb, subinfo);
> +
> +        if (mkv->mode != MODE_WEBM) {
> +            mkv_write_dovi(s, pb, st);
> +        }
> +
>          break;
>  
>      case AVMEDIA_TYPE_AUDIO:
> 

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v10 3/5] avformat/mov: Refactor mov_read_dvcc_dvvc to use ff_isom_parse_dvcc_dvvc

[Andreas Rheinhardt]

quietvoid:
> To avoid duplicating code. The implementation in dovi_isom is identical.
> 
> Signed-off-by: quietvoid <tcChlisop0@gmail.com>
> ---
>  libavformat/mov.c | 52 ++++++++---------------------------------------
>  1 file changed, 9 insertions(+), 43 deletions(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 9ebfa0bcc7..e3c80c399c 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -55,6 +55,7 @@
>  #include "avformat.h"
>  #include "internal.h"
>  #include "avio_internal.h"
> +#include "dovi_isom.h"
>  #include "riff.h"
>  #include "isom.h"
>  #include "libavcodec/get_bits.h"
> @@ -7056,58 +7057,23 @@ static int mov_read_dmlp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  static int mov_read_dvcc_dvvc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  {
>      AVStream *st;
> -    uint32_t buf;
> -    AVDOVIDecoderConfigurationRecord *dovi;
> -    size_t dovi_size;
> +    uint8_t buf[ISOM_DVCC_DVVC_SIZE];
>      int ret;
> +    int64_t read_size = atom.size;
>  
>      if (c->fc->nb_streams < 1)
>          return 0;
>      st = c->fc->streams[c->fc->nb_streams-1];
>  
> -    if ((uint64_t)atom.size > (1<<30) || atom.size < 4)
> -        return AVERROR_INVALIDDATA;
> -
> -    dovi = av_dovi_alloc(&dovi_size);
> -    if (!dovi)
> -        return AVERROR(ENOMEM);
> -
> -    dovi->dv_version_major = avio_r8(pb);
> -    dovi->dv_version_minor = avio_r8(pb);
> -
> -    buf = avio_rb16(pb);
> -    dovi->dv_profile        = (buf >> 9) & 0x7f;    // 7 bits
> -    dovi->dv_level          = (buf >> 3) & 0x3f;    // 6 bits
> -    dovi->rpu_present_flag  = (buf >> 2) & 0x01;    // 1 bit
> -    dovi->el_present_flag   = (buf >> 1) & 0x01;    // 1 bit
> -    dovi->bl_present_flag   =  buf       & 0x01;    // 1 bit
> -    if (atom.size >= 24) {  // 4 + 4 + 4 * 4
> -        buf = avio_r8(pb);
> -        dovi->dv_bl_signal_compatibility_id = (buf >> 4) & 0x0f; // 4 bits
> -    } else {
> -        // 0 stands for None
> -        // Dolby Vision V1.2.93 profiles and levels
> -        dovi->dv_bl_signal_compatibility_id = 0;
> +    // At most 24 bytes
> +    if (read_size > ISOM_DVCC_DVVC_SIZE) {
> +        read_size = ISOM_DVCC_DVVC_SIZE;
>      }

Unnecessary parentheses. (Could even be made to read_size =
FFMIN(read_size, ISOM_DVCC_DVVC_SIZE); to save another line.)

>  
> -    ret = av_stream_add_side_data(st, AV_PKT_DATA_DOVI_CONF,
> -                                  (uint8_t *)dovi, dovi_size);
> -    if (ret < 0) {
> -        av_free(dovi);
> -        return ret;
> -    }
> +    if ((ret = avio_read(pb, buf, read_size)) != read_size)

Could use ffio_read_size() (a wrapper around avio_read() that returns an
error if the desired amount of data could not be read completely).

> +        return ret < 0 ? ret : AVERROR(EIO);
>  
> -    av_log(c, AV_LOG_TRACE, "DOVI in dvcC/dvvC/dvwC box, version: %d.%d, profile: %d, level: %d, "
> -           "rpu flag: %d, el flag: %d, bl flag: %d, compatibility id: %d\n",
> -           dovi->dv_version_major, dovi->dv_version_minor,
> -           dovi->dv_profile, dovi->dv_level,
> -           dovi->rpu_present_flag,
> -           dovi->el_present_flag,
> -           dovi->bl_present_flag,
> -           dovi->dv_bl_signal_compatibility_id
> -        );
> -
> -    return 0;
> +    return ff_isom_parse_dvcc_dvvc(c->fc, st, buf, read_size);
>  }
>  
>  static int mov_read_kind(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> 

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v10 1/5] avformat/dovi_isom: Implement Dolby Vision configuration parsing/writing

[Andreas Rheinhardt]

quietvoid:
> Both parse/write implementations are based on mov/movenc.
> 
> This only adds support for the "Dolby Vision configuration box".
> Other configuration boxes, such as
> "Dolby Vision enhancement layer configuration box" are not supported.
> 
> The new functions will be used to implement parsing/writing the DOVI config
> for Matroska, as well as to refactor both mov/movenc to use dovi_isom functions.
> 
> Signed-off-by: quietvoid <tcChlisop0@gmail.com>
> ---
>  libavformat/dovi_isom.c | 118 ++++++++++++++++++++++++++++++++++++++++
>  libavformat/dovi_isom.h |  35 ++++++++++++
>  2 files changed, 153 insertions(+)
>  create mode 100644 libavformat/dovi_isom.c
>  create mode 100644 libavformat/dovi_isom.h
> 
> diff --git a/libavformat/dovi_isom.c b/libavformat/dovi_isom.c
> new file mode 100644
> index 0000000000..262d7e37ba
> --- /dev/null
> +++ b/libavformat/dovi_isom.c
> @@ -0,0 +1,118 @@
> +/*
> + * DOVI ISO Media common code
> + *
> + * Copyright (c) 2020 Vacing Fang <vacingfang@tencent.com>
> + * Copyright (c) 2021 quietvoid
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "libavutil/dovi_meta.h"
> +
> +#include "libavcodec/put_bits.h"
> +
> +#include "avformat.h"
> +#include "dovi_isom.h"
> +
> +int ff_isom_parse_dvcc_dvvc(AVFormatContext *s, AVStream *st, const uint8_t *buf_ptr, uint64_t size)
> +{
> +    uint32_t buf;
> +    AVDOVIDecoderConfigurationRecord *dovi;
> +    size_t dovi_size;
> +    int ret;
> +
> +    if (size > (1 << 30) || size < 4)
> +        return AVERROR_INVALIDDATA;
> +
> +    dovi = av_dovi_alloc(&dovi_size);
> +    if (!dovi)
> +        return AVERROR(ENOMEM);
> +
> +    dovi->dv_version_major = *buf_ptr++;    // 8 bits
> +    dovi->dv_version_minor = *buf_ptr++;    // 8 bits
> +
> +    buf = *buf_ptr++ << 8;
> +    buf |= *buf_ptr++;
> +
> +    dovi->dv_profile        = (buf >> 9) & 0x7f;    // 7 bits
> +    dovi->dv_level          = (buf >> 3) & 0x3f;    // 6 bits
> +    dovi->rpu_present_flag  = (buf >> 2) & 0x01;    // 1 bit
> +    dovi->el_present_flag   = (buf >> 1) & 0x01;    // 1 bit
> +    dovi->bl_present_flag   =  buf       & 0x01;    // 1 bit
> +
> +    // Has enough remaining data
> +    if (size >= 5) {
> +        dovi->dv_bl_signal_compatibility_id = ((*buf_ptr++) >> 4) & 0x0f; // 4 bits
> +    } else {
> +        // 0 stands for None
> +        // Dolby Vision V1.2.93 profiles and levels
> +        dovi->dv_bl_signal_compatibility_id = 0;
> +    }
> +
> +    ret = av_stream_add_side_data(st, AV_PKT_DATA_DOVI_CONF,
> +                                  (uint8_t *)dovi, dovi_size);
> +    if (ret < 0) {
> +        av_free(dovi);
> +        return ret;
> +    }
> +
> +    av_log(s, AV_LOG_TRACE, "DOVI in dvcC/dvvC/dvwC box, version: %d.%d, profile: %d, level: %d, "
> +           "rpu flag: %d, el flag: %d, bl flag: %d, compatibility id: %d\n",
> +           dovi->dv_version_major, dovi->dv_version_minor,
> +           dovi->dv_profile, dovi->dv_level,
> +           dovi->rpu_present_flag,
> +           dovi->el_present_flag,
> +           dovi->bl_present_flag,
> +           dovi->dv_bl_signal_compatibility_id);
> +
> +    return 0;
> +}
> +
> +void ff_isom_put_dvcc_dvvc(AVFormatContext *s, uint8_t out[ISOM_DVCC_DVVC_SIZE],
> +                           AVDOVIDecoderConfigurationRecord *dovi)
> +{
> +    PutBitContext pb;
> +
> +    init_put_bits(&pb, out, ISOM_DVCC_DVVC_SIZE);
> +
> +    put_bits(&pb, 8, dovi->dv_version_major);
> +    put_bits(&pb, 8, dovi->dv_version_minor);
> +    put_bits(&pb, 7, dovi->dv_profile);
> +    put_bits(&pb, 6, dovi->dv_level);
> +    put_bits(&pb, 1, dovi->rpu_present_flag);
> +    put_bits(&pb, 1, dovi->el_present_flag);
> +    put_bits(&pb, 1, dovi->bl_present_flag);
> +    put_bits(&pb, 4, dovi->dv_bl_signal_compatibility_id);

There is a slight problem with the last six values: They come from the
user and therefore must not be trusted to be within the range they are
supposed to be. (The put_bits API uses an av_assert2 to check that
the values to be written actually fit into the amount of bits that is
used to write them.) So I suggest you use !! for the flags and
appropriate masks for dv_profile, dv_level and
dv_bl_signal_compatibility_id.

> +
> +    put_bits(&pb, 28, 0); /* reserved */
> +    put_bits32(&pb, 0); /* reserved */
> +    put_bits32(&pb, 0); /* reserved */
> +    put_bits32(&pb, 0); /* reserved */
> +    put_bits32(&pb, 0); /* reserved */
> +
> +    flush_put_bits(&pb);
> +
> +    av_log(s, AV_LOG_DEBUG, "DOVI in %s box, version: %d.%d, profile: %d, level: %d, "
> +           "rpu flag: %d, el flag: %d, bl flag: %d, compatibility id: %d\n",
> +           dovi->dv_profile > 10 ? "dvwC" : (dovi->dv_profile > 7 ? "dvvC" : "dvcC"),
> +           dovi->dv_version_major, dovi->dv_version_minor,
> +           dovi->dv_profile, dovi->dv_level,
> +           dovi->rpu_present_flag,
> +           dovi->el_present_flag,
> +           dovi->bl_present_flag,
> +           dovi->dv_bl_signal_compatibility_id);
> +}
> diff --git a/libavformat/dovi_isom.h b/libavformat/dovi_isom.h
> new file mode 100644
> index 0000000000..1526164319
> --- /dev/null
> +++ b/libavformat/dovi_isom.h
> @@ -0,0 +1,35 @@
> +/*
> + * DOVI ISO Media common code
> + * Copyright (c) 2021 quietvoid
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVFORMAT_DOVI_ISOM_H
> +#define AVFORMAT_DOVI_ISOM_H
> +
> +#include "libavutil/dovi_meta.h"
> +
> +#include "avformat.h"
> +
> +#define ISOM_DVCC_DVVC_SIZE 24
> +
> +int ff_isom_parse_dvcc_dvvc(AVFormatContext *s, AVStream *st, const uint8_t *buf_ptr, uint64_t size);
> +void ff_isom_put_dvcc_dvvc(AVFormatContext *s, uint8_t out[ISOM_DVCC_DVVC_SIZE],
> +                           AVDOVIDecoderConfigurationRecord *dovi);
> +
> +#endif /* AVFORMAT_DOVI_ISOM_H */
> 

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v10 1/5] avformat/dovi_isom: Implement Dolby Vision configuration parsing/writing

[quietvoid]

On Fri, Dec 31, 2021 at 9:16 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> quietvoid:
> > Both parse/write implementations are based on mov/movenc.
> >
> > This only adds support for the "Dolby Vision configuration box".
> > Other configuration boxes, such as
> > "Dolby Vision enhancement layer configuration box" are not supported.
> >
> > The new functions will be used to implement parsing/writing the DOVI config
> > for Matroska, as well as to refactor both mov/movenc to use dovi_isom functions.
> >
> > Signed-off-by: quietvoid <tcChlisop0@gmail.com>
> > ---
> >  libavformat/dovi_isom.c | 118 ++++++++++++++++++++++++++++++++++++++++
> >  libavformat/dovi_isom.h |  35 ++++++++++++
> >  2 files changed, 153 insertions(+)
> >  create mode 100644 libavformat/dovi_isom.c
> >  create mode 100644 libavformat/dovi_isom.h
> >
> > diff --git a/libavformat/dovi_isom.c b/libavformat/dovi_isom.c
> > new file mode 100644
> > index 0000000000..262d7e37ba
> > --- /dev/null
> > +++ b/libavformat/dovi_isom.c
> > @@ -0,0 +1,118 @@
> > +/*
> > + * DOVI ISO Media common code
> > + *
> > + * Copyright (c) 2020 Vacing Fang <vacingfang@tencent.com>
> > + * Copyright (c) 2021 quietvoid
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +#include "libavutil/dovi_meta.h"
> > +
> > +#include "libavcodec/put_bits.h"
> > +
> > +#include "avformat.h"
> > +#include "dovi_isom.h"
> > +
> > +int ff_isom_parse_dvcc_dvvc(AVFormatContext *s, AVStream *st, const uint8_t *buf_ptr, uint64_t size)
> > +{
> > +    uint32_t buf;
> > +    AVDOVIDecoderConfigurationRecord *dovi;
> > +    size_t dovi_size;
> > +    int ret;
> > +
> > +    if (size > (1 << 30) || size < 4)
> > +        return AVERROR_INVALIDDATA;
> > +
> > +    dovi = av_dovi_alloc(&dovi_size);
> > +    if (!dovi)
> > +        return AVERROR(ENOMEM);
> > +
> > +    dovi->dv_version_major = *buf_ptr++;    // 8 bits
> > +    dovi->dv_version_minor = *buf_ptr++;    // 8 bits
> > +
> > +    buf = *buf_ptr++ << 8;
> > +    buf |= *buf_ptr++;
> > +
> > +    dovi->dv_profile        = (buf >> 9) & 0x7f;    // 7 bits
> > +    dovi->dv_level          = (buf >> 3) & 0x3f;    // 6 bits
> > +    dovi->rpu_present_flag  = (buf >> 2) & 0x01;    // 1 bit
> > +    dovi->el_present_flag   = (buf >> 1) & 0x01;    // 1 bit
> > +    dovi->bl_present_flag   =  buf       & 0x01;    // 1 bit
> > +
> > +    // Has enough remaining data
> > +    if (size >= 5) {
> > +        dovi->dv_bl_signal_compatibility_id = ((*buf_ptr++) >> 4) & 0x0f; // 4 bits
> > +    } else {
> > +        // 0 stands for None
> > +        // Dolby Vision V1.2.93 profiles and levels
> > +        dovi->dv_bl_signal_compatibility_id = 0;
> > +    }
> > +
> > +    ret = av_stream_add_side_data(st, AV_PKT_DATA_DOVI_CONF,
> > +                                  (uint8_t *)dovi, dovi_size);
> > +    if (ret < 0) {
> > +        av_free(dovi);
> > +        return ret;
> > +    }
> > +
> > +    av_log(s, AV_LOG_TRACE, "DOVI in dvcC/dvvC/dvwC box, version: %d.%d, profile: %d, level: %d, "
> > +           "rpu flag: %d, el flag: %d, bl flag: %d, compatibility id: %d\n",
> > +           dovi->dv_version_major, dovi->dv_version_minor,
> > +           dovi->dv_profile, dovi->dv_level,
> > +           dovi->rpu_present_flag,
> > +           dovi->el_present_flag,
> > +           dovi->bl_present_flag,
> > +           dovi->dv_bl_signal_compatibility_id);
> > +
> > +    return 0;
> > +}
> > +
> > +void ff_isom_put_dvcc_dvvc(AVFormatContext *s, uint8_t out[ISOM_DVCC_DVVC_SIZE],
> > +                           AVDOVIDecoderConfigurationRecord *dovi)
> > +{
> > +    PutBitContext pb;
> > +
> > +    init_put_bits(&pb, out, ISOM_DVCC_DVVC_SIZE);
> > +
> > +    put_bits(&pb, 8, dovi->dv_version_major);
> > +    put_bits(&pb, 8, dovi->dv_version_minor);
> > +    put_bits(&pb, 7, dovi->dv_profile);
> > +    put_bits(&pb, 6, dovi->dv_level);
> > +    put_bits(&pb, 1, dovi->rpu_present_flag);
> > +    put_bits(&pb, 1, dovi->el_present_flag);
> > +    put_bits(&pb, 1, dovi->bl_present_flag);
> > +    put_bits(&pb, 4, dovi->dv_bl_signal_compatibility_id);
>
> There is a slight problem with the last six values: They come from the
> user and therefore must not be trusted to be within the range they are
> supposed to be. (The put_bits API uses an av_assert2 to check that
> the values to be written actually fit into the amount of bits that is
> used to write them.) So I suggest you use !! for the flags and
> appropriate masks for dv_profile, dv_level and
> dv_bl_signal_compatibility_id.

Thanks. I used the same masks as in the read function.

Updated along with the fixes for the other patches in v11:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-January/290611.html

> > +
> > +    put_bits(&pb, 28, 0); /* reserved */
> > +    put_bits32(&pb, 0); /* reserved */
> > +    put_bits32(&pb, 0); /* reserved */
> > +    put_bits32(&pb, 0); /* reserved */
> > +    put_bits32(&pb, 0); /* reserved */
> > +
> > +    flush_put_bits(&pb);
> > +
> > +    av_log(s, AV_LOG_DEBUG, "DOVI in %s box, version: %d.%d, profile: %d, level: %d, "
> > +           "rpu flag: %d, el flag: %d, bl flag: %d, compatibility id: %d\n",
> > +           dovi->dv_profile > 10 ? "dvwC" : (dovi->dv_profile > 7 ? "dvvC" : "dvcC"),
> > +           dovi->dv_version_major, dovi->dv_version_minor,
> > +           dovi->dv_profile, dovi->dv_level,
> > +           dovi->rpu_present_flag,
> > +           dovi->el_present_flag,
> > +           dovi->bl_present_flag,
> > +           dovi->dv_bl_signal_compatibility_id);
> > +}
> > diff --git a/libavformat/dovi_isom.h b/libavformat/dovi_isom.h
> > new file mode 100644
> > index 0000000000..1526164319
> > --- /dev/null
> > +++ b/libavformat/dovi_isom.h
> > @@ -0,0 +1,35 @@
> > +/*
> > + * DOVI ISO Media common code
> > + * Copyright (c) 2021 quietvoid
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +#ifndef AVFORMAT_DOVI_ISOM_H
> > +#define AVFORMAT_DOVI_ISOM_H
> > +
> > +#include "libavutil/dovi_meta.h"
> > +
> > +#include "avformat.h"
> > +
> > +#define ISOM_DVCC_DVVC_SIZE 24
> > +
> > +int ff_isom_parse_dvcc_dvvc(AVFormatContext *s, AVStream *st, const uint8_t *buf_ptr, uint64_t size);
> > +void ff_isom_put_dvcc_dvvc(AVFormatContext *s, uint8_t out[ISOM_DVCC_DVVC_SIZE],
> > +                           AVDOVIDecoderConfigurationRecord *dovi);
> > +
> > +#endif /* AVFORMAT_DOVI_ISOM_H */
> >
>
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v10 1/5] avformat/dovi_isom: Implement Dolby Vision configuration parsing/writing

[Andreas Rheinhardt]

quietvoid:
> On Fri, Dec 31, 2021 at 9:16 PM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
>>
>> quietvoid:
>>> Both parse/write implementations are based on mov/movenc.
>>>
>>> This only adds support for the "Dolby Vision configuration box".
>>> Other configuration boxes, such as
>>> "Dolby Vision enhancement layer configuration box" are not supported.
>>>
>>> The new functions will be used to implement parsing/writing the DOVI config
>>> for Matroska, as well as to refactor both mov/movenc to use dovi_isom functions.
>>>
>>> Signed-off-by: quietvoid <tcChlisop0@gmail.com>
>>> ---
>>>  libavformat/dovi_isom.c | 118 ++++++++++++++++++++++++++++++++++++++++
>>>  libavformat/dovi_isom.h |  35 ++++++++++++
>>>  2 files changed, 153 insertions(+)
>>>  create mode 100644 libavformat/dovi_isom.c
>>>  create mode 100644 libavformat/dovi_isom.h
>>>
>>> diff --git a/libavformat/dovi_isom.c b/libavformat/dovi_isom.c
>>> new file mode 100644
>>> index 0000000000..262d7e37ba
>>> --- /dev/null
>>> +++ b/libavformat/dovi_isom.c
>>> @@ -0,0 +1,118 @@
>>> +/*
>>> + * DOVI ISO Media common code
>>> + *
>>> + * Copyright (c) 2020 Vacing Fang <vacingfang@tencent.com>
>>> + * Copyright (c) 2021 quietvoid
>>> + *
>>> + * This file is part of FFmpeg.
>>> + *
>>> + * FFmpeg is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2.1 of the License, or (at your option) any later version.
>>> + *
>>> + * FFmpeg is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with FFmpeg; if not, write to the Free Software
>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>>> + */
>>> +
>>> +#include "libavutil/dovi_meta.h"
>>> +
>>> +#include "libavcodec/put_bits.h"
>>> +
>>> +#include "avformat.h"
>>> +#include "dovi_isom.h"
>>> +
>>> +int ff_isom_parse_dvcc_dvvc(AVFormatContext *s, AVStream *st, const uint8_t *buf_ptr, uint64_t size)
>>> +{
>>> +    uint32_t buf;
>>> +    AVDOVIDecoderConfigurationRecord *dovi;
>>> +    size_t dovi_size;
>>> +    int ret;
>>> +
>>> +    if (size > (1 << 30) || size < 4)
>>> +        return AVERROR_INVALIDDATA;
>>> +
>>> +    dovi = av_dovi_alloc(&dovi_size);
>>> +    if (!dovi)
>>> +        return AVERROR(ENOMEM);
>>> +
>>> +    dovi->dv_version_major = *buf_ptr++;    // 8 bits
>>> +    dovi->dv_version_minor = *buf_ptr++;    // 8 bits
>>> +
>>> +    buf = *buf_ptr++ << 8;
>>> +    buf |= *buf_ptr++;
>>> +
>>> +    dovi->dv_profile        = (buf >> 9) & 0x7f;    // 7 bits
>>> +    dovi->dv_level          = (buf >> 3) & 0x3f;    // 6 bits
>>> +    dovi->rpu_present_flag  = (buf >> 2) & 0x01;    // 1 bit
>>> +    dovi->el_present_flag   = (buf >> 1) & 0x01;    // 1 bit
>>> +    dovi->bl_present_flag   =  buf       & 0x01;    // 1 bit
>>> +
>>> +    // Has enough remaining data
>>> +    if (size >= 5) {
>>> +        dovi->dv_bl_signal_compatibility_id = ((*buf_ptr++) >> 4) & 0x0f; // 4 bits
>>> +    } else {
>>> +        // 0 stands for None
>>> +        // Dolby Vision V1.2.93 profiles and levels
>>> +        dovi->dv_bl_signal_compatibility_id = 0;
>>> +    }
>>> +
>>> +    ret = av_stream_add_side_data(st, AV_PKT_DATA_DOVI_CONF,
>>> +                                  (uint8_t *)dovi, dovi_size);
>>> +    if (ret < 0) {
>>> +        av_free(dovi);
>>> +        return ret;
>>> +    }
>>> +
>>> +    av_log(s, AV_LOG_TRACE, "DOVI in dvcC/dvvC/dvwC box, version: %d.%d, profile: %d, level: %d, "
>>> +           "rpu flag: %d, el flag: %d, bl flag: %d, compatibility id: %d\n",
>>> +           dovi->dv_version_major, dovi->dv_version_minor,
>>> +           dovi->dv_profile, dovi->dv_level,
>>> +           dovi->rpu_present_flag,
>>> +           dovi->el_present_flag,
>>> +           dovi->bl_present_flag,
>>> +           dovi->dv_bl_signal_compatibility_id);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +void ff_isom_put_dvcc_dvvc(AVFormatContext *s, uint8_t out[ISOM_DVCC_DVVC_SIZE],
>>> +                           AVDOVIDecoderConfigurationRecord *dovi)
>>> +{
>>> +    PutBitContext pb;
>>> +
>>> +    init_put_bits(&pb, out, ISOM_DVCC_DVVC_SIZE);
>>> +
>>> +    put_bits(&pb, 8, dovi->dv_version_major);
>>> +    put_bits(&pb, 8, dovi->dv_version_minor);
>>> +    put_bits(&pb, 7, dovi->dv_profile);
>>> +    put_bits(&pb, 6, dovi->dv_level);
>>> +    put_bits(&pb, 1, dovi->rpu_present_flag);
>>> +    put_bits(&pb, 1, dovi->el_present_flag);
>>> +    put_bits(&pb, 1, dovi->bl_present_flag);
>>> +    put_bits(&pb, 4, dovi->dv_bl_signal_compatibility_id);
>>
>> There is a slight problem with the last six values: They come from the
>> user and therefore must not be trusted to be within the range they are
>> supposed to be. (The put_bits API uses an av_assert2 to check that
>> the values to be written actually fit into the amount of bits that is
>> used to write them.) So I suggest you use !! for the flags and
>> appropriate masks for dv_profile, dv_level and
>> dv_bl_signal_compatibility_id.
> 
> Thanks. I used the same masks as in the read function.
> 

The current code is also dangerous (just a bit less than this version of
your patchset): If dv_profile & 128 is set (which it mustn't), then the
avio_wb16() will run into the av_assert2 in avio_w8() (if assert-level
is >=2).
(And dovi->dv_bl_signal_compatibility_id << 28 would be UB in case
dv_bl_signal_compatibility_id & 0xF8 is != 0.)

- Andreas
_______________________________________________
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".