diff mbox series

[FFmpeg-devel,v2,3/5] avformat/matroskaenc: Write dvcC/dvvC block additional mapping

Message ID 20210923050109.692356-3-tcChlisop0@gmail.com
State New
Headers show
Series [FFmpeg-devel,v2,1/5] avformat/matroskadec: Parse Block Additional Mapping elements
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

quietvoid Sept. 23, 2021, 5:01 a.m. UTC
When muxing to Matroska, write the Block Additional Mapping
if there is AV_PKT_DATA_DOVI_CONF side data present.
Most of the code was implemented by Plex developers.

Signed-off-by: quietvoid <tcChlisop0@gmail.com>
---
 libavformat/Makefile      |  2 +-
 libavformat/dovi_isom.c   | 45 +++++++++++++++++++++++++++++++++++++++
 libavformat/dovi_isom.h   |  5 +++++
 libavformat/matroskaenc.c | 32 ++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt Sept. 23, 2021, 5:33 a.m. UTC | #1
quietvoid:
> When muxing to Matroska, write the Block Additional Mapping
> if there is AV_PKT_DATA_DOVI_CONF side data present.
> Most of the code was implemented by Plex developers.
> 
> Signed-off-by: quietvoid <tcChlisop0@gmail.com>
> ---
>  libavformat/Makefile      |  2 +-
>  libavformat/dovi_isom.c   | 45 +++++++++++++++++++++++++++++++++++++++
>  libavformat/dovi_isom.h   |  5 +++++
>  libavformat/matroskaenc.c | 32 ++++++++++++++++++++++++++++
>  4 files changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 61a1fecf6c..680030014d 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -317,7 +317,7 @@ OBJS-$(CONFIG_MATROSKA_DEMUXER)          += matroskadec.o matroska.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/dovi_isom.c b/libavformat/dovi_isom.c
> index 3eafa0d13b..15acc5cbce 100644
> --- a/libavformat/dovi_isom.c
> +++ b/libavformat/dovi_isom.c
> @@ -19,6 +19,8 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>  
> +#include "libavcodec/put_bits.h"
> +

The typical order is libavutil-libavcodec-libavformat headers.

>  #include "libavutil/dovi_meta.h"
>  
>  #include "avformat.h"
> @@ -77,4 +79,47 @@ int ff_isom_parse_dvcc_dvvc(AVFormatContext *s, AVStream *st, const uint8_t *buf
>          );
>  
>      return 0;
> +}
> +
> +int ff_isom_put_dvcc_dvvc(AVFormatContext *s, uint8_t *out, int size, uint32_t *type,

You can use uint8_t out[ISOM_DVCC_DVVC_SIZE] to make it clear that you
expect users to provide you with a buffer of this size.

> +                         AVDOVIDecoderConfigurationRecord *dovi)
> +{
> +    PutBitContext pb;
> +    init_put_bits(&pb, out, size);
> +
> +    if (size < ISOM_DVCC_DVVC_SIZE)
> +        return AVERROR(EINVAL);
> +
> +    if (dovi->dv_profile > 7)
> +        *type = MKBETAG('d', 'v', 'v', 'C');
> +    else
> +        *type = MKBETAG('d', 'v', 'c', 'C');
> +
> +    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);
> +
> +    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 > 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);
> +
> +    return put_bytes_output(&pb);
>  }
> \ No newline at end of file

All C source files need to end with an empty line (i.e. they need to end
with a newline). So just add it here and also in the header.

> diff --git a/libavformat/dovi_isom.h b/libavformat/dovi_isom.h
> index 9d9a05bdb0..b58ab1bea0 100644
> --- a/libavformat/dovi_isom.h
> +++ b/libavformat/dovi_isom.h
> @@ -23,7 +23,12 @@
>  #define AVFORMAT_DOVI_ISOM_H
>  
>  #include "avformat.h"
> +#include "libavutil/dovi_meta.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);
> +int ff_isom_put_dvcc_dvvc(AVFormatContext *s, uint8_t *out, int size, uint32_t *type,
> +                         AVDOVIDecoderConfigurationRecord *dovi);
>  
>  #endif /* AVFORMAT_DOVI_ISOM_H */
> \ No newline at end of file
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 039f20988a..a54f7b5feb 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -54,6 +54,8 @@
>  #include "libavcodec/xiph.h"
>  #include "libavcodec/mpeg4audio.h"
>  
> +#include "libavformat/dovi_isom.h"
> +
>  /* Level 1 elements we create a SeekHead entry for:
>   * Info, Tracks, Chapters, Attachments, Tags (potentially twice) and Cues */
>  #define MAX_SEEKHEAD_ENTRIES 7
> @@ -1115,6 +1117,33 @@ static int mkv_write_stereo_mode(AVFormatContext *s, AVIOContext *pb,
>      return 0;
>  }
>  
> +static int mkv_write_dovi(AVFormatContext *s, AVIOContext *pb, AVStream *st)
> +{
> +    int ret;
> +    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;
> +        int size;
> +
> +        if ((ret = ff_isom_put_dvcc_dvvc(s, buf, sizeof(buf), &type, dovi)) < 0)
> +            return ret;
> +
> +        size = ret;
> +
> +        mapping = start_ebml_master(pb, MATROSKA_ID_TRACKBLKADDMAPPING, ISOM_DVCC_DVVC_SIZE);

This is wrong: ISOM_DVCC_DVVC_SIZE need not be an upper bound for the
size of the blockgroup. (2 + 1 + strlen("Dolby Vision configuration")) +
(2 + 1 + 4) + (2 + 1 + ISOM_DVCC_DVVC_SIZE) is -- although I'd prefer if
you used sizeof("Dolby Vision configuration") - 1 instead of strlen.

> +        put_ebml_string(pb, MATROSKA_ID_BLKADDIDNAME, "Dolby Vision configuration");
> +        put_ebml_uint(pb, MATROSKA_ID_BLKADDIDTYPE, type);
> +        put_ebml_binary(pb, MATROSKA_ID_BLKADDIDEXTRADATA, buf, size);
> +        end_ebml_master(pb, mapping);
> +    }
> +
> +    return 0;
> +}
> +
>  static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>                             AVStream *st, mkv_track *track, AVIOContext *pb,
>                             int is_default)
> @@ -1380,6 +1409,9 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>          return AVERROR(EINVAL);
>      }
>  
> +    if ((ret = mkv_write_dovi(s, pb, st)) < 0)
> +        return ret;

Given that this is not part of WebM, there needs to be check for
mkv->mode != MODE_WEBM. Furthermore, this should also be moved to the
AVMEDIA_TYPE_VIDEO switch statement directly after the video master
element has been closed.

> +
>      if (mkv->mode != MODE_WEBM || par->codec_id != AV_CODEC_ID_WEBVTT) {
>          track->codecpriv_offset = avio_tell(pb);
>          ret = mkv_write_codecprivate(s, pb, par, native_id, qt_id);
>
quietvoid Sept. 23, 2021, 12:45 p.m. UTC | #2
On 23/09/2021 01.33, Andreas Rheinhardt wrote:
>> diff --git a/libavformat/dovi_isom.h b/libavformat/dovi_isom.h
>> index 9d9a05bdb0..b58ab1bea0 100644
>> --- a/libavformat/dovi_isom.h
>> +++ b/libavformat/dovi_isom.h
>> @@ -23,7 +23,12 @@
>>   #define AVFORMAT_DOVI_ISOM_H
>>   
>>   #include "avformat.h"
>> +#include "libavutil/dovi_meta.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);
>> +int ff_isom_put_dvcc_dvvc(AVFormatContext *s, uint8_t *out, int size, uint32_t *type,
>> +                         AVDOVIDecoderConfigurationRecord *dovi);
>>   
>>   #endif /* AVFORMAT_DOVI_ISOM_H */
>> \ No newline at end of file
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 039f20988a..a54f7b5feb 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -54,6 +54,8 @@
>>   #include "libavcodec/xiph.h"
>>   #include "libavcodec/mpeg4audio.h"
>>   
>> +#include "libavformat/dovi_isom.h"
>> +
>>   /* Level 1 elements we create a SeekHead entry for:
>>    * Info, Tracks, Chapters, Attachments, Tags (potentially twice) and Cues */
>>   #define MAX_SEEKHEAD_ENTRIES 7
>> @@ -1115,6 +1117,33 @@ static int mkv_write_stereo_mode(AVFormatContext *s, AVIOContext *pb,
>>       return 0;
>>   }
>>   
>> +static int mkv_write_dovi(AVFormatContext *s, AVIOContext *pb, AVStream *st)
>> +{
>> +    int ret;
>> +    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;
>> +        int size;
>> +
>> +        if ((ret = ff_isom_put_dvcc_dvvc(s, buf, sizeof(buf), &type, dovi)) < 0)
>> +            return ret;
>> +
>> +        size = ret;
>> +
>> +        mapping = start_ebml_master(pb, MATROSKA_ID_TRACKBLKADDMAPPING, ISOM_DVCC_DVVC_SIZE);
> This is wrong: ISOM_DVCC_DVVC_SIZE need not be an upper bound for the
> size of the blockgroup. (2 + 1 + strlen("Dolby Vision configuration")) +
> (2 + 1 + 4) + (2 + 1 + ISOM_DVCC_DVVC_SIZE) is -- although I'd prefer if
> you used sizeof("Dolby Vision configuration") - 1 instead of strlen.
Okay, I was confused on what you meant previously.
I have changed it to use this variable:
uint64_t expected_size = (2 + 1 + (sizeof("Dolby Vision configuration") 
- 1))
                                              + (2 + 1 + 4) + (2 + 1 + 
ISOM_DVCC_DVVC_SIZE);

>> +        put_ebml_string(pb, MATROSKA_ID_BLKADDIDNAME, "Dolby Vision configuration");
>> +        put_ebml_uint(pb, MATROSKA_ID_BLKADDIDTYPE, type);
>> +        put_ebml_binary(pb, MATROSKA_ID_BLKADDIDEXTRADATA, buf, size);
>> +        end_ebml_master(pb, mapping);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>>                              AVStream *st, mkv_track *track, AVIOContext *pb,
>>                              int is_default)
>> @@ -1380,6 +1409,9 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>>           return AVERROR(EINVAL);
>>       }
>>   
>> +    if ((ret = mkv_write_dovi(s, pb, st)) < 0)
>> +        return ret;
> Given that this is not part of WebM, there needs to be check for
> mkv->mode != MODE_WEBM. Furthermore, this should also be moved to the
> AVMEDIA_TYPE_VIDEO switch statement directly after the video master
> element has been closed.

Corrected, thank you.


Is it OK to resend a new version or should I wait in case more
changes are necessary in the other patches?
diff mbox series

Patch

diff --git a/libavformat/Makefile b/libavformat/Makefile
index 61a1fecf6c..680030014d 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -317,7 +317,7 @@  OBJS-$(CONFIG_MATROSKA_DEMUXER)          += matroskadec.o matroska.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/dovi_isom.c b/libavformat/dovi_isom.c
index 3eafa0d13b..15acc5cbce 100644
--- a/libavformat/dovi_isom.c
+++ b/libavformat/dovi_isom.c
@@ -19,6 +19,8 @@ 
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include "libavcodec/put_bits.h"
+
 #include "libavutil/dovi_meta.h"
 
 #include "avformat.h"
@@ -77,4 +79,47 @@  int ff_isom_parse_dvcc_dvvc(AVFormatContext *s, AVStream *st, const uint8_t *buf
         );
 
     return 0;
+}
+
+int ff_isom_put_dvcc_dvvc(AVFormatContext *s, uint8_t *out, int size, uint32_t *type,
+                         AVDOVIDecoderConfigurationRecord *dovi)
+{
+    PutBitContext pb;
+    init_put_bits(&pb, out, size);
+
+    if (size < ISOM_DVCC_DVVC_SIZE)
+        return AVERROR(EINVAL);
+
+    if (dovi->dv_profile > 7)
+        *type = MKBETAG('d', 'v', 'v', 'C');
+    else
+        *type = MKBETAG('d', 'v', 'c', 'C');
+
+    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);
+
+    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 > 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);
+
+    return put_bytes_output(&pb);
 }
\ No newline at end of file
diff --git a/libavformat/dovi_isom.h b/libavformat/dovi_isom.h
index 9d9a05bdb0..b58ab1bea0 100644
--- a/libavformat/dovi_isom.h
+++ b/libavformat/dovi_isom.h
@@ -23,7 +23,12 @@ 
 #define AVFORMAT_DOVI_ISOM_H
 
 #include "avformat.h"
+#include "libavutil/dovi_meta.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);
+int ff_isom_put_dvcc_dvvc(AVFormatContext *s, uint8_t *out, int size, uint32_t *type,
+                         AVDOVIDecoderConfigurationRecord *dovi);
 
 #endif /* AVFORMAT_DOVI_ISOM_H */
\ No newline at end of file
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 039f20988a..a54f7b5feb 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -54,6 +54,8 @@ 
 #include "libavcodec/xiph.h"
 #include "libavcodec/mpeg4audio.h"
 
+#include "libavformat/dovi_isom.h"
+
 /* Level 1 elements we create a SeekHead entry for:
  * Info, Tracks, Chapters, Attachments, Tags (potentially twice) and Cues */
 #define MAX_SEEKHEAD_ENTRIES 7
@@ -1115,6 +1117,33 @@  static int mkv_write_stereo_mode(AVFormatContext *s, AVIOContext *pb,
     return 0;
 }
 
+static int mkv_write_dovi(AVFormatContext *s, AVIOContext *pb, AVStream *st)
+{
+    int ret;
+    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;
+        int size;
+
+        if ((ret = ff_isom_put_dvcc_dvvc(s, buf, sizeof(buf), &type, dovi)) < 0)
+            return ret;
+
+        size = ret;
+
+        mapping = start_ebml_master(pb, MATROSKA_ID_TRACKBLKADDMAPPING, ISOM_DVCC_DVVC_SIZE);
+        put_ebml_string(pb, MATROSKA_ID_BLKADDIDNAME, "Dolby Vision configuration");
+        put_ebml_uint(pb, MATROSKA_ID_BLKADDIDTYPE, type);
+        put_ebml_binary(pb, MATROSKA_ID_BLKADDIDEXTRADATA, buf, size);
+        end_ebml_master(pb, mapping);
+    }
+
+    return 0;
+}
+
 static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
                            AVStream *st, mkv_track *track, AVIOContext *pb,
                            int is_default)
@@ -1380,6 +1409,9 @@  static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
         return AVERROR(EINVAL);
     }
 
+    if ((ret = mkv_write_dovi(s, pb, st)) < 0)
+        return ret;
+
     if (mkv->mode != MODE_WEBM || par->codec_id != AV_CODEC_ID_WEBVTT) {
         track->codecpriv_offset = avio_tell(pb);
         ret = mkv_write_codecprivate(s, pb, par, native_id, qt_id);