| 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 | expand |
| 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 |
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); >
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 --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);
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(-)