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

2021-09-22 Thread Andreas Rheinhardt
quietvoid:
> 
> On 22/09/2021 22.03, Andreas Rheinhardt wrote:
>> quietvoid:
>>> +    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 +1407,12 @@ static int mkv_write_track(AVFormatContext *s,
>>> MatroskaMuxContext *mkv,
>>>   return AVERROR(EINVAL);
>>>   }
>>>   +    // After video track, before private data
>> Is this required?
> 
> No, if DOVI side data can be assumed to always be along a video track.
> Which is the case, I think.
> 
> The order is not necessary but this is the same behaviour as other
> Matroska muxers.
> I've simply validated the elements with mkvinfo.
> 

Then please remove the comment. It suggests that this order is required
when it is not.

- 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-requ...@ffmpeg.org with subject "unsubscribe".


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

2021-09-22 Thread quietvoid



On 22/09/2021 22.03, Andreas Rheinhardt wrote:

quietvoid:

+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 +1407,12 @@ static int mkv_write_track(AVFormatContext *s, 
MatroskaMuxContext *mkv,
  return AVERROR(EINVAL);
  }
  
+// After video track, before private data

Is this required?


No, if DOVI side data can be assumed to always be along a video track.
Which is the case, I think.

The order is not necessary but this is the same behaviour as other 
Matroska muxers.

I've simply validated the elements with mkvinfo.


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


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

2021-09-22 Thread Andreas Rheinhardt
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 
> ---
>  libavformat/isom.c| 42 +++
>  libavformat/isom.h|  4 
>  libavformat/matroskaenc.c | 33 ++
>  3 files changed, 79 insertions(+)
> 
> diff --git a/libavformat/isom.c b/libavformat/isom.c
> index 80a8f4d7b0..a5eb2e0caf 100644
> --- a/libavformat/isom.c
> +++ b/libavformat/isom.c
> @@ -481,3 +481,45 @@ int ff_mov_parse_dvcc_dvvc(AVFormatContext *s, AVStream 
> *st, GetBitContext *gb)
>  
>  return 0;
>  }
> +
> +int ff_mov_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 < MOV_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_bits_count(&pb) / 8;

return put_bytes_output(&pb);

> +}
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index 44bd839852..6214180c00 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -28,6 +28,7 @@
>  #include 
>  
>  #include "libavcodec/get_bits.h"
> +#include "libavcodec/put_bits.h"

This header should be included in the file with ff_mov_put_dvcc_dvvc,
not in isom.h.

>  
>  #include "libavutil/dovi_meta.h"
>  #include "libavutil/encryption_info.h"
> @@ -393,6 +394,9 @@ static inline enum AVCodecID ff_mov_get_lpcm_codec_id(int 
> bps, int flags)
>  #define MOV_ISMV_TTML_TAG MKTAG('d', 'f', 'x', 'p')
>  #define MOV_MP4_TTML_TAG  MKTAG('s', 't', 'p', 'p')
>  
> +#define MOV_DVCC_DVVC_SIZE 24
>  int ff_mov_parse_dvcc_dvvc(AVFormatContext *s, AVStream *st, GetBitContext 
> *gb);
> +int ff_mov_put_dvcc_dvvc(AVFormatContext *s, uint8_t *out, int size, 
> uint32_t *type,
> + AVDOVIDecoderConfigurationRecord *dovi);
>  
>  #endif /* AVFORMAT_ISOM_H */
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 039f20988a..e99bd655cc 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1115,6 +1115,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[MOV_DVCC_DVVC_SIZE];
> +uint32_t type;
> +int size;
> +
> +if ((ret = ff_mov_put_dvcc_dvvc(s, buf, sizeof(buf), &type, dovi)) < 
> 0)
> +return ret;
> +
> +size = ret;
> +
> +mapping = start_ebml_master(pb, MATROSKA_ID_TRACKBLKADDMAPPING, 0);

You actually have a good upper bound for the final size; one that allows
to write this master element using a one-byte length field.

> +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 +1407,12 @@ static int mkv_write