Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: fix sample size being zero in pcmC
On 7/26/23 11:22, Zhao Zhili wrote: From: ffmpeg-devel On Behalf Of Raphaël Zumer Sent: 2023年7月26日 21:38 To: ffmpeg-devel@ffmpeg.org Subject: Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: fix sample size being zero in pcmC On 7/25/23 23:45, Zhao Zhili wrote: From: Zhao Zhili -avio_w8(pb, track->par->bits_per_raw_sample); +sample_size = track->par->bits_per_raw_sample; +if (!sample_size) +sample_size = av_get_exact_bits_per_sample(track->par->codec_id); +av_assert0(sample_size); +avio_w8(pb, sample_size); Why not just replace bits_per_raw_sample completely with av_get_exact_bits_per_sample()? Are there edge cases, like incorrect codec ID, or sample sizes smaller than specified by the codec ID (and if there is ever a mismatch, which is expected in pcmC?) If not, there should be no need to prioritize bits_per_raw_sample in the first place and add a fallback - going straight to av_get_exact_bits_per_sample() should suffice and keep the code simpler. Check bits_per_raw_sample first for a little bit of performance. Don't use bits_per_coded_sample because it make less sense in this case and to keep the code simpler. I doubt it is worth the trouble, but my opinion is not important if no one else cares. As noted in another thread, bits_per_raw_sample seems to always be 0 when muxing PCM to MP4 with FFmpeg, whereas bits_per_coded_sample has the correct value from the samples that I tested. That's only one usecase. bits_per_coded_sample has been set while bit_per_raw_sample wasn't only because the implementation of wav demux and there's no encoding process. (You can try -c:a pcm_s16le) There are more usecases to support, e.g., only use mp4 muxer without any demuxer or encoder. User can create and set a AVCodecParameters manually, and let bits_per_coded_sample be zero. That is fair, I just wanted to point out the mismatch as a potential (separate) issue. RZ ___ 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] avformat/movenc: Fix writing a sample size of 0 for PCM in MP4
On 7/26/23 11:37, Zhao Zhili wrote: I think you mixed up PCM in mp4 (-f mp4) and PCM in quicktime format (-f mov). There is no support of mux PCM in mp4 before the patch. And demux PCM in mp4 only works by chance. Let's focus on improve current situation, there isn't much to look back. I double-checked my test sample that I thought was muxed with FFmpeg 6.0, and actually the lavf version matches the most recent one. FFmpeg 6.0 does reject it for muxing in MP4. Sorry for the mixup. RZ ___ 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 1/2] avformat/movenc: fix sample size being zero in pcmC
On 7/25/23 23:45, Zhao Zhili wrote: From: Zhao Zhili -avio_w8(pb, track->par->bits_per_raw_sample); +sample_size = track->par->bits_per_raw_sample; +if (!sample_size) +sample_size = av_get_exact_bits_per_sample(track->par->codec_id); +av_assert0(sample_size); +avio_w8(pb, sample_size); Why not just replace bits_per_raw_sample completely with av_get_exact_bits_per_sample()? Are there edge cases, like incorrect codec ID, or sample sizes smaller than specified by the codec ID (and if there is ever a mismatch, which is expected in pcmC?) If not, there should be no need to prioritize bits_per_raw_sample in the first place and add a fallback - going straight to av_get_exact_bits_per_sample() should suffice and keep the code simpler. As noted in another thread, bits_per_raw_sample seems to always be 0 when muxing PCM to MP4 with FFmpeg, whereas bits_per_coded_sample has the correct value from the samples that I tested. Raphaël Zumer ___ 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] avformat/movenc: Fix writing a sample size of 0 for PCM in MP4
Hello, On 7/26/23 00:09, "zhilizhao(赵志立)" wrote: On Jul 26, 2023, at 00:28, Raphaël Zumer wrote: Encoding PCM in MP4 currently causes subsequent decoding to fail due to a sample size of 0. This doesn’t give a context on which case the sample size is 0. Using FFmpeg (round-trip). Just mux some Wave file to MP4 and try decoding it back. Unless it is sample-dependent for some reason, it will always fail. Use bits per coded sample instead, which are set correctly based on my tests and allow muxed files to be decoded as expected. Since neither bits_per_raw_sample or bits_per_coded_sample has strong guarantee, I decided to fall back to av_get_exact_bits_per_sample(). http://ffmpeg.org/pipermail/ffmpeg-devel/2023-July/312653.html Can you show or describe a case where bits_per_raw_sample is set and not bits_per_coded_sample? For PCM av_get_exact_bits_per_sample() looks fine, but if the behavior of bits_per_raw_sample/bits_per_coded_sample is inconsistent on decode that seems undesirable or undocumented behavior that should be rectified. Looking at the documentation (https://ffmpeg.org/doxygen/6.0/structAVCodecParameters.html) it is mentioned to be equivalent to bits_per_raw_sample for PCM formats. So this looks like a bug. Note: PCM in MP4 muxed with versions of FFmpeg 6.0 and prior (before implementation of the pcmC box) will continue to fail decoding due to the sample size not being available. I see that it was assumed to be 16-bit before commit d4ee17. The assumption is incorrect. We didn’t recognize those samples are ISO/IEC 23003-5 before. I mean that it is possible to mux s16 PCM into MP4 (in a nonstandard way I guess) prior to that patch set and this breaks decoding files created this way. I can mux a s16le Wave file using FFmpeg 6.0 to MP4, it will be decoded correctly and retain the sample format. Try to decode it using FFmpeg post the 23003-5 patch and it simply fails decoding because there's no pcmC box. There doesn't need to be a fallback, just a consideration. Thanks Raphaël Zumer ___ 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] [RFC] [PATCH] avcodec/codec_par: Keep format channel layout if specified
Thanks for the reply; I wasn't looking to merge the RFC as-is, but rather to figure out the preferred approach, and most importantly make sure that there is no fundamental disagreement on suppressing codec-level metadata in situations where it conflicts with format-level. I will see which of the alternative implementations comes out cleaner (*probably* a separate function due to the multiple call locations) and either send a v2 RFC patch or separate set later. Raphaël Zumer On 7/25/23 14:53, Anton Khirnov wrote: Quoting Vittorio Giovara Any comments on this patch? If no objections I'd like to push it at the end of the week Sorry, not acceptable. This is the wrong place to do it. AVCodecParameters is a dumb container for parameters. It MUST NOT make any assumptions about who calls it or for what purpose. The caller tells it to copy data - it copies data. Sadly I don't have the time to think about this in depth right now (ask me again in 3 weeks or so), but some potential alternatives: * handle this explicitly in the caller * add a new function, say avcodec_parameters_update(), perhaps with a flags parameter controlling how exactly is the update to be performed; not entirely sure this generalizes well enough ___ 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".
[FFmpeg-devel] [PATCH] avformat/movenc: Fix writing a sample size of 0 for PCM in MP4
Encoding PCM in MP4 currently causes subsequent decoding to fail due to a sample size of 0. Use bits per coded sample instead, which are set correctly based on my tests and allow muxed files to be decoded as expected. Note: PCM in MP4 muxed with versions of FFmpeg 6.0 and prior (before implementation of the pcmC box) will continue to fail decoding due to the sample size not being available. I see that it was assumed to be 16-bit before commit d4ee17. Signed-off-by: Raphaël Zumer --- libavformat/movenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/movenc.c b/libavformat/movenc.c index f1cc80b1b3..3c44ace5b0 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -1237,7 +1237,7 @@ static int mov_write_pcmc_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra track->par->codec_id == AV_CODEC_ID_PCM_S24LE || track->par->codec_id == AV_CODEC_ID_PCM_S32LE); avio_w8(pb, format_flags); -avio_w8(pb, track->par->bits_per_raw_sample); +avio_w8(pb, track->par->bits_per_coded_sample); return update_size(pb, pos); } -- 2.41.0 ___ 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".
[FFmpeg-devel] [RFC] [PATCH] avcodec/codec_par: Keep format channel layout if specified
Hello,
Tagging this as RFC in case there is disagreement on the correct/desired
behavior.
For context:
I am working with spatial audio and noticed that FFmpeg does not honor the SA3D
box metadata, which is used to signal ambisonic audio channel layouts.
FFmpeg does parse the SA3D box and applies the specified channel layout
properties to the codec parameters. However, they are later reset based on
codec-level parameters in avcodec_parameters_from_context(), which is called
from multiple locations between SA3D being parsed and the channel layout being
resolved in FFprobe.
The result is that only codecs which support ambisonic signaling (I don't know
of any besides Opus) can be recognized as ambisonic, despite container-level
metadata being present. Since a lot of ambisonic content is distributed in AAC
or PCM/Wave format, I think that the SA3D metadata should take precedence over
codec-level metadata in this case.
Obviously this means ignoring the codec-level metadata which may not match. If
there are reasons to prioritize the channel layout defined at the codec level
at the expense of SA3D, please let me know the rationale. Also, I am aware that
at least libswresample is also ignoring the container-level metadata, so other
changes may be needed to propagate the SA3D properties when decoding/filtering
ambisonic tracks, but this allows for detection as a first step.
The implementation could be greatly simplified by not resetting the channel
layout, but that would require removing av_channel_layout_uninit() from
codec_parameters_reset() or adding an argument to skip it. I would favor the
former, but this affects a few other functions in this file so let me know if
you have a preference.
I assume this would be a micro version bump (or minor?) but will add that once
other details are sorted out.
Thanks,
Raphaël Zumer
Signed-off-by: Raphaël Zumer
---
libavcodec/codec_par.c | 32 +---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/libavcodec/codec_par.c b/libavcodec/codec_par.c
index 775c187073..0cd707d431 100644
--- a/libavcodec/codec_par.c
+++ b/libavcodec/codec_par.c
@@ -100,10 +100,34 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const
AVCodecParameters *src
int avcodec_parameters_from_context(AVCodecParameters *par,
const AVCodecContext *codec)
{
+int keep_ch_layout = par->ch_layout.order != AV_CHANNEL_ORDER_UNSPEC;
+AVChannelLayout ch_layout;
int ret;
+/*
+ * The stream codec parameters may have a channel layout set
+ * already that is not represented in the codec context.
+ * For example, spatial audio channel layouts in codecs with no
+ * signaling for them may be decoded from container-level metadata.
+ *
+ * Assume that if the channel order is specified, we should
+ * preserve the existing layout rather than let
+ * avcodec_parameters_from_context() override it.
+ */
+if (keep_ch_layout) {
+ret = av_channel_layout_copy(_layout, >ch_layout);
+if (ret < 0)
+return ret;
+}
+
codec_parameters_reset(par);
+if (keep_ch_layout) {
+ret = av_channel_layout_copy(>ch_layout, _layout);
+if (ret < 0)
+return ret;
+}
+
par->codec_type = codec->codec_type;
par->codec_id = codec->codec_id;
par->codec_tag = codec->codec_tag;
@@ -146,9 +170,11 @@ FF_DISABLE_DEPRECATION_WARNINGS
FF_ENABLE_DEPRECATION_WARNINGS
} else {
#endif
-ret = av_channel_layout_copy(>ch_layout, >ch_layout);
-if (ret < 0)
-return ret;
+if (!keep_ch_layout) {
+ret = av_channel_layout_copy(>ch_layout, >ch_layout);
+if (ret < 0)
+return ret;
+}
#if FF_API_OLD_CHANNEL_LAYOUT
FF_DISABLE_DEPRECATION_WARNINGS
}
--
2.41.0
___
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".
[FFmpeg-devel] [PATCH v7 2/2] avutil: add HDR10+ dynamic metadata serialization function
Co-authored-by: Mohammad Izadi
Signed-off-by: Raphaël Zumer
---
doc/APIchanges | 5 ++
libavutil/hdr_dynamic_metadata.c | 148 +++
libavutil/hdr_dynamic_metadata.h | 13 +++
libavutil/version.h | 2 +-
4 files changed, 167 insertions(+), 1 deletion(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 14737223cb..3a61d61931 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,11 @@ The last version increases of all libraries were on 2023-02-09
API changes, most recent first:
+2023-03-13 - xx - lavu 58.4.100 - hdr_dynamic_metadata.h
+ Add av_dynamic_hdr_plus_from_t35() and av_dynamic_hdr_plus_to_t35()
+ functions to convert between raw T.35 payloads containing dynamic
+ HDR10+ metadata and their parsed representations as AVDynamicHDRPlus.
+
2023-03-02 - xx - lavc 60.6.100 - avcodec.h
Add FF_PROFILE_EAC3_DDP_ATMOS, FF_PROFILE_TRUEHD_ATMOS,
FF_PROFILE_DTS_HD_MA_X and FF_PROFILE_DTS_HD_MA_X_IMAX.
diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 5ed903f475..b26eafc5f2 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -20,6 +20,7 @@
#include "hdr_dynamic_metadata.h"
#include "mem.h"
+#include "libavcodec/defs.h"
#include "libavcodec/get_bits.h"
#include "libavcodec/put_bits.h"
@@ -230,3 +231,150 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s,
const uint8_t *data,
return 0;
}
+
+int av_dynamic_hdr_plus_to_t35(const AVDynamicHDRPlus *s, uint8_t **data,
size_t *size)
+{
+uint8_t *buf;
+size_t size_bits, size_bytes;
+PutBitContext pbc, *pb =
+
+if (!s || !data)
+return AVERROR(EINVAL);
+
+/**
+ * Buffer size per CTA-861-H p.253-254:
+ * 48 header bits (excluded from the serialized payload)
+ * 8 bits for application_mode
+ * 2 bits for num_windows
+ * 153 bits for window geometry, for each window above 1
+ * 27 bits for targeted_system_display_maximum_luminance
+ * 1-2511 bits for targeted system display peak luminance information
+ * 82-442 bits per window for pixel distribution information
+ * 1-2511 bits for mastering display peak luminance information
+ * 1-179 bits per window for tonemapping information
+ * 1-7 bits per window for color saturation mapping information
+ * Total: 123-7249 bits, excluding trimmed header bits
+ */
+size_bits = 8;
+
+size_bits += 2;
+
+for (int w = 1; w < s->num_windows; w++)
+size_bits += 153;
+
+size_bits += 27;
+
+size_bits += 1;
+if (s->targeted_system_display_actual_peak_luminance_flag)
+size_bits += 10 +
+ s->num_rows_targeted_system_display_actual_peak_luminance
*
+ s->num_cols_targeted_system_display_actual_peak_luminance
* 4;
+
+for (int w = 0; w < s->num_windows; w++)
+size_bits += 72 + s->params[w].num_distribution_maxrgb_percentiles *
24 + 10;
+
+size_bits += 1;
+if (s->mastering_display_actual_peak_luminance_flag)
+size_bits += 10 +
+ s->num_rows_mastering_display_actual_peak_luminance *
+ s->num_cols_mastering_display_actual_peak_luminance * 4;
+
+for (int w = 0; w < s->num_windows; w++) {
+size_bits += 1;
+if (s->params[w].tone_mapping_flag)
+size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+
+size_bits += 1;
+if (s->params[w].color_saturation_mapping_flag)
+size_bits += 6;
+}
+
+size_bytes = (size_bits + 7) / 8;
+
+buf = av_malloc(size_bytes);
+if (!buf)
+return AVERROR(ENOMEM);
+
+init_put_bits(pb, buf, size_bytes);
+
+// application_mode is set to Application Version 1
+put_bits(pb, 8, 1);
+
+// Payload as per CTA-861-H p.253-254
+put_bits(pb, 2, s->num_windows);
+
+for (int w = 1; w < s->num_windows; w++) {
+put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num /
s->params[w].window_upper_left_corner_x.den);
+put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num /
s->params[w].window_upper_left_corner_y.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num /
s->params[w].window_lower_right_corner_x.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num /
s->params[w].window_lower_right_corner_y.den);
+put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+put_bits(pb, 8, s->params[w].rotation_angle);
+put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+put_bits(pb, 16, s->params[w].semiminor_axis_external_ell
[FFmpeg-devel] [PATCH v7 1/2] avcodec/avutil: move dynamic HDR10+ metadata parsing to libavutil
Signed-off-by: Raphaël Zumer --- libavcodec/Makefile | 6 +- libavcodec/av1dec.c | 6 +- libavcodec/dynamic_hdr10_plus.c | 198 --- libavcodec/dynamic_hdr10_plus.h | 35 -- libavcodec/h2645_sei.c | 6 +- libavcodec/libdav1d.c| 6 +- libavutil/hdr_dynamic_metadata.c | 185 + libavutil/hdr_dynamic_metadata.h | 13 ++ 8 files changed, 210 insertions(+), 245 deletions(-) delete mode 100644 libavcodec/dynamic_hdr10_plus.c delete mode 100644 libavcodec/dynamic_hdr10_plus.h diff --git a/libavcodec/Makefile b/libavcodec/Makefile index abae4909d2..408ecd1e31 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -104,7 +104,7 @@ OBJS-$(CONFIG_H264_SEI)+= h264_sei.o h2645_sei.o OBJS-$(CONFIG_HEVCPARSE) += hevc_parse.o hevc_ps.o hevc_data.o \ h2645data.o h2645_parse.o h2645_vui.o OBJS-$(CONFIG_HEVC_SEI)+= hevc_sei.o h2645_sei.o \ - dynamic_hdr10_plus.o dynamic_hdr_vivid.o + dynamic_hdr_vivid.o OBJS-$(CONFIG_HPELDSP) += hpeldsp.o OBJS-$(CONFIG_HUFFMAN) += huffman.o OBJS-$(CONFIG_HUFFYUVDSP) += huffyuvdsp.o @@ -250,7 +250,7 @@ OBJS-$(CONFIG_ATRAC3PAL_DECODER) += atrac3plusdec.o atrac3plus.o \ OBJS-$(CONFIG_ATRAC9_DECODER) += atrac9dec.o OBJS-$(CONFIG_AURA_DECODER)+= cyuv.o OBJS-$(CONFIG_AURA2_DECODER) += aura.o -OBJS-$(CONFIG_AV1_DECODER) += av1dec.o dynamic_hdr10_plus.o +OBJS-$(CONFIG_AV1_DECODER) += av1dec.o OBJS-$(CONFIG_AV1_CUVID_DECODER) += cuviddec.o OBJS-$(CONFIG_AV1_MEDIACODEC_DECODER) += mediacodecdec.o OBJS-$(CONFIG_AV1_NVENC_ENCODER) += nvenc_av1.o nvenc.o @@ -1082,7 +1082,7 @@ OBJS-$(CONFIG_LIBARIBB24_DECODER) += libaribb24.o ass.o OBJS-$(CONFIG_LIBCELT_DECODER)+= libcelt_dec.o OBJS-$(CONFIG_LIBCODEC2_DECODER) += libcodec2.o OBJS-$(CONFIG_LIBCODEC2_ENCODER) += libcodec2.o -OBJS-$(CONFIG_LIBDAV1D_DECODER) += libdav1d.o dynamic_hdr10_plus.o +OBJS-$(CONFIG_LIBDAV1D_DECODER) += libdav1d.o OBJS-$(CONFIG_LIBDAVS2_DECODER) += libdavs2.o OBJS-$(CONFIG_LIBFDK_AAC_DECODER) += libfdk-aacdec.o OBJS-$(CONFIG_LIBFDK_AAC_ENCODER) += libfdk-aacenc.o diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index a80e37e33f..df393fe3d0 100644 --- a/libavcodec/av1dec.c +++ b/libavcodec/av1dec.c @@ -20,6 +20,7 @@ #include "config_components.h" +#include "libavutil/hdr_dynamic_metadata.h" #include "libavutil/film_grain_params.h" #include "libavutil/mastering_display_metadata.h" #include "libavutil/pixdesc.h" @@ -30,7 +31,6 @@ #include "bytestream.h" #include "codec_internal.h" #include "decode.h" -#include "dynamic_hdr10_plus.h" #include "hwconfig.h" #include "profiles.h" #include "thread.h" @@ -925,8 +925,8 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame, if (!hdrplus) return AVERROR(ENOMEM); -ret = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer, - bytestream2_get_bytes_left()); +ret = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer, + bytestream2_get_bytes_left()); if (ret < 0) return ret; break; diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c deleted file mode 100644 index 34a44aac65..00 --- a/libavcodec/dynamic_hdr10_plus.c +++ /dev/null @@ -1,198 +0,0 @@ -/* - * 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 "dynamic_hdr10_plus.h" -#include "get_bits.h" - -static const int64_t luminance_den = 1; -static const int32_t peak_luminance_den = 15; -static const int64_t rgb_den = 10; -static const int32_t fraction_pixel_den = 1000; -static const int32_t knee_point_den = 4095; -static const int32_t
Re: [FFmpeg-devel] [PATCH v6 1/2] avcodec/avutil: move dynamic HDR10+ metadata parsing to libavutil
On 3/13/23 20:44, Andreas Rheinhardt wrote: > Looking at the calculation in av_dynamic_hdr_plus_to_t35(), it seems > that the maximum bitlength of a valid ITU-T T.35 payload is > 48+2×937+27+1+10+25×25×4+3×82+(3×15×24)+(1+10+25×25×4+3×1)+(3×(28+15×10))+3+3×6 > = 8855 bits (please double-check this). This means we can just copy that > much into a padded buffer on the stack and ignore the padding. We may > then remove the padding from the serialization function, too. I went over the calculations again and found a couple of places where size was overestimated due to fields that can fit larger values than their specified maximum. 937 also seems like a typo on my part (unsure how that happened, maybe multiplied instead of added some number), it should be 153. I re-ran the numbers for everything and rewrote/expanded the size calculation code somewhat to make reading alongside the spec or block comment easier. I also removed another 40 bits from the header to match the parsing function, which begins parsing at the application mode, rather than at the terminal provider code. That means we exclude 48 bits in total from the header, and the calculation should be as follows: 8 + 2 + 2 * 153 + 27 + 1 + 10 + 25 * 25 * 4 + 3 * (72 + 15 * 24 + 10) + 1 + 10 + 25 * 25 * 4 + 3 * (1 + 28 + 15 * 10) + 3 * (1 + 6) = 7249 bits maximum (907 bytes rounded up). Can we correct the padding amount along with get_bits.h in a separate patch? I would like to avoid scope creep. Will send a new version with the changes mentioned above. I've tested the round trip (HEVC decode -> parse -> serialize -> HEVC encode). RZ ___ 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 v6 1/2] avcodec/avutil: move dynamic HDR10+ metadata parsing to libavutil
On 3/13/23 20:44, Andreas Rheinhardt wrote: >> diff --git a/libavutil/hdr_dynamic_metadata.h >> b/libavutil/hdr_dynamic_metadata.h >> index 2d72de56ae..3d327241c1 100644 >> --- a/libavutil/hdr_dynamic_metadata.h >> +++ b/libavutil/hdr_dynamic_metadata.h >> @@ -340,4 +340,15 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t >> *size); >> */ >> AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame); >> >> +/** >> + * Parse the user data registered ITU-T T.35 to AVbuffer (AVDynamicHDRPlus). >> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure. >> + * @param data The byte array containing the raw ITU-T T.35 data. >> + * @param size Size of the data array in bytes. > The implementation of this function uses the GetBit-API which requires > the buffer to be padded; yet the documentation does not mention this. > > Looking at the calculation in av_dynamic_hdr_plus_to_t35(), it seems > that the maximum bitlength of a valid ITU-T T.35 payload is > 48+2×937+27+1+10+25×25×4+3×82+(3×15×24)+(1+10+25×25×4+3×1)+(3×(28+15×10))+3+3×6 > = 8855 bits (please double-check this). This means we can just copy that > much into a padded buffer on the stack and ignore the padding. We may > then remove the padding from the serialization function, too. > > (The GetBit-API btw actually does not need AV_INPUT_BUFFER_PADDING_SIZE > bytes of padding, but way less (I think 8 bytes or so). I don't really > like using AV_INPUT_BUFFER_PADDING_SIZE here.) Hi, From get_bits.h (init_get_bits()): "buffer, must be AV_INPUT_BUFFER_PADDING_SIZE bytes larger than the actual read bits". Is this wrong? If so, what is the correct value and is it defined anywhere? What about "other places where one needed a padded buffer" as mentioned in your previous comment? Why is copying the buffer to the stack in the parsing function preferable to padding it when serializing? The vast majority of HDR10+ payloads will be well below the max size. RZ ___ 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".
[FFmpeg-devel] [PATCH v6 2/2] avutil: add HDR10+ dynamic metadata serialization function
Co-authored-by: Mohammad Izadi
Signed-off-by: Raphaël Zumer
---
doc/APIchanges | 5 ++
libavutil/hdr_dynamic_metadata.c | 148 +++
libavutil/hdr_dynamic_metadata.h | 13 +++
libavutil/version.h | 2 +-
4 files changed, 167 insertions(+), 1 deletion(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 14737223cb..3a61d61931 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,11 @@ The last version increases of all libraries were on 2023-02-09
API changes, most recent first:
+2023-03-13 - xx - lavu 58.4.100 - hdr_dynamic_metadata.h
+ Add av_dynamic_hdr_plus_from_t35() and av_dynamic_hdr_plus_to_t35()
+ functions to convert between raw T.35 payloads containing dynamic
+ HDR10+ metadata and their parsed representations as AVDynamicHDRPlus.
+
2023-03-02 - xx - lavc 60.6.100 - avcodec.h
Add FF_PROFILE_EAC3_DDP_ATMOS, FF_PROFILE_TRUEHD_ATMOS,
FF_PROFILE_DTS_HD_MA_X and FF_PROFILE_DTS_HD_MA_X_IMAX.
diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 223db2fd66..da3c5f4947 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -20,6 +20,7 @@
#include "hdr_dynamic_metadata.h"
#include "mem.h"
+#include "libavcodec/defs.h"
#include "libavcodec/get_bits.h"
#include "libavcodec/put_bits.h"
@@ -225,3 +226,150 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s,
const uint8_t *data,
return 0;
}
+
+int av_dynamic_hdr_plus_to_t35(const AVDynamicHDRPlus *s, uint8_t **data,
size_t *size)
+{
+uint8_t *buf;
+size_t size_bits, size_bytes;
+PutBitContext pbc, *pb =
+
+if (!s || !data)
+return AVERROR(EINVAL);
+
+/**
+ * Buffer size per CTA-861-H p.253-254:
+ * 48 bits for the header (56 minus excluded 8-bit country code)
+ * 2 bits for num_windows
+ * 937 bits for window geometry, for each window above 1
+ * 27 bits for targeted_system_display_maximum_luminance
+ * 1-3855 bits for targeted system display peak luminance information
+ * 0-442 bits for intra-window pixel distribution information
+ * 1-3855 bits for mastering display peak luminance information
+ * 0-537 bits for per-window tonemapping information
+ * 0-21 bits for per-window color saturation mapping information
+ */
+size_bits = 48 +
+2 +
+FFMAX((s->num_windows - 1), 0) * 937 +
+27 +
+1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_targeted_system_display_actual_peak_luminance *
+s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0)
+
+s->num_windows * 82;
+
+for (int w = 0; w < s->num_windows; w++)
+size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+
+size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_mastering_display_actual_peak_luminance *
+s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+s->num_windows * 1;
+
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].tone_mapping_flag)
+size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+}
+
+size_bits += s->num_windows * 1;
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].color_saturation_mapping_flag)
+size_bits += 6;
+}
+
+size_bytes = (size_bits + 7) / 8;
+
+// Pad the allocated buffer to account for optimized readers that may use
it directly.
+buf = av_malloc(size_bytes + AV_INPUT_BUFFER_PADDING_SIZE);
+if (!buf)
+return AVERROR(ENOMEM);
+
+init_put_bits(pb, buf, size_bytes);
+
+// itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
+// itu_t_t35_terminal_provider_code shall be 0x003C
+put_bits(pb, 16, 0x003C);
+// itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+put_bits(pb, 16, 0x0001);
+// application_identifier shall be set to 4
+put_bits(pb, 8, 4);
+// application_mode is set to Application Version 1
+put_bits(pb, 8, 1);
+
+// Payload as per CTA-861-H p.253-254
+put_bits(pb, 2, s->num_windows);
+
+for (int w = 1; w < s->num_windows; w++) {
+put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num /
s->params[w].window_upper_left_corner_x.den);
+put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num /
s->params[w].window_upper_left_corner_y.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num /
s->params[w].window_lower_right_corner_x.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num /
s->params[w].window_lower_right_corner_y.den);
+put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+
[FFmpeg-devel] [PATCH v6 1/2] avcodec/avutil: move dynamic HDR10+ metadata parsing to libavutil
Signed-off-by: Raphaël Zumer --- libavcodec/Makefile | 6 +- libavcodec/av1dec.c | 6 +- libavcodec/dynamic_hdr10_plus.c | 198 --- libavcodec/dynamic_hdr10_plus.h | 35 -- libavcodec/h2645_sei.c | 6 +- libavcodec/libdav1d.c| 6 +- libavutil/hdr_dynamic_metadata.c | 180 libavutil/hdr_dynamic_metadata.h | 11 ++ 8 files changed, 203 insertions(+), 245 deletions(-) delete mode 100644 libavcodec/dynamic_hdr10_plus.c delete mode 100644 libavcodec/dynamic_hdr10_plus.h diff --git a/libavcodec/Makefile b/libavcodec/Makefile index abae4909d2..408ecd1e31 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -104,7 +104,7 @@ OBJS-$(CONFIG_H264_SEI)+= h264_sei.o h2645_sei.o OBJS-$(CONFIG_HEVCPARSE) += hevc_parse.o hevc_ps.o hevc_data.o \ h2645data.o h2645_parse.o h2645_vui.o OBJS-$(CONFIG_HEVC_SEI)+= hevc_sei.o h2645_sei.o \ - dynamic_hdr10_plus.o dynamic_hdr_vivid.o + dynamic_hdr_vivid.o OBJS-$(CONFIG_HPELDSP) += hpeldsp.o OBJS-$(CONFIG_HUFFMAN) += huffman.o OBJS-$(CONFIG_HUFFYUVDSP) += huffyuvdsp.o @@ -250,7 +250,7 @@ OBJS-$(CONFIG_ATRAC3PAL_DECODER) += atrac3plusdec.o atrac3plus.o \ OBJS-$(CONFIG_ATRAC9_DECODER) += atrac9dec.o OBJS-$(CONFIG_AURA_DECODER)+= cyuv.o OBJS-$(CONFIG_AURA2_DECODER) += aura.o -OBJS-$(CONFIG_AV1_DECODER) += av1dec.o dynamic_hdr10_plus.o +OBJS-$(CONFIG_AV1_DECODER) += av1dec.o OBJS-$(CONFIG_AV1_CUVID_DECODER) += cuviddec.o OBJS-$(CONFIG_AV1_MEDIACODEC_DECODER) += mediacodecdec.o OBJS-$(CONFIG_AV1_NVENC_ENCODER) += nvenc_av1.o nvenc.o @@ -1082,7 +1082,7 @@ OBJS-$(CONFIG_LIBARIBB24_DECODER) += libaribb24.o ass.o OBJS-$(CONFIG_LIBCELT_DECODER)+= libcelt_dec.o OBJS-$(CONFIG_LIBCODEC2_DECODER) += libcodec2.o OBJS-$(CONFIG_LIBCODEC2_ENCODER) += libcodec2.o -OBJS-$(CONFIG_LIBDAV1D_DECODER) += libdav1d.o dynamic_hdr10_plus.o +OBJS-$(CONFIG_LIBDAV1D_DECODER) += libdav1d.o OBJS-$(CONFIG_LIBDAVS2_DECODER) += libdavs2.o OBJS-$(CONFIG_LIBFDK_AAC_DECODER) += libfdk-aacdec.o OBJS-$(CONFIG_LIBFDK_AAC_ENCODER) += libfdk-aacenc.o diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index a80e37e33f..df393fe3d0 100644 --- a/libavcodec/av1dec.c +++ b/libavcodec/av1dec.c @@ -20,6 +20,7 @@ #include "config_components.h" +#include "libavutil/hdr_dynamic_metadata.h" #include "libavutil/film_grain_params.h" #include "libavutil/mastering_display_metadata.h" #include "libavutil/pixdesc.h" @@ -30,7 +31,6 @@ #include "bytestream.h" #include "codec_internal.h" #include "decode.h" -#include "dynamic_hdr10_plus.h" #include "hwconfig.h" #include "profiles.h" #include "thread.h" @@ -925,8 +925,8 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame, if (!hdrplus) return AVERROR(ENOMEM); -ret = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer, - bytestream2_get_bytes_left()); +ret = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer, + bytestream2_get_bytes_left()); if (ret < 0) return ret; break; diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c deleted file mode 100644 index 34a44aac65..00 --- a/libavcodec/dynamic_hdr10_plus.c +++ /dev/null @@ -1,198 +0,0 @@ -/* - * 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 "dynamic_hdr10_plus.h" -#include "get_bits.h" - -static const int64_t luminance_den = 1; -static const int32_t peak_luminance_den = 15; -static const int64_t rgb_den = 10; -static const int32_t fraction_pixel_den = 1000; -static const int32_t knee_point_den = 4095; -static const int32_t
Re: [FFmpeg-devel] [PATCH v5 2/2] avutil: add HDR10+ dynamic metadata serialization function
On 3/13/23 19:25, James Almer wrote: >>> You are allocating without any padding. This implies that one could not >>> use this buffer with our GetBit-API or in other places where one needed >>> a padded buffer. >> Is there any comparable code that does that? I feel like padding a buffer >> should be the responsibility of the caller for a public function, otherwise >> the user has to be aware of the padding to avoid embedding extra payload >> bytes accidentally (even though it is negligible in size), it is an extra >> manipulation if padding is not needed, and requires including an extra file >> to access the padding size. > The returned value in *size would not take the padding bytes into > account, so no way to include them accidentally anywhere. You either > know the size of the serialized data and trust the buffer is complete as > Andreas mentioned, or you read the size returned by the function. In > either case, the padding bytes are never considered. Right, I did not think it through. Will do. RZ ___ 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 v5 2/2] avutil: add HDR10+ dynamic metadata serialization function
On 3/13/23 18:35, Andreas Rheinhardt wrote: > size being mandatory is different from similar APIs. There is even a > usecase without size: If you simply feed this to something that expects > the data to be serialized and trust the data to be complete, you don't > need the size. OK, I'll amend that. > You are allocating without any padding. This implies that one could not > use this buffer with our GetBit-API or in other places where one needed > a padded buffer. Is there any comparable code that does that? I feel like padding a buffer should be the responsibility of the caller for a public function, otherwise the user has to be aware of the padding to avoid embedding extra payload bytes accidentally (even though it is negligible in size), it is an extra manipulation if padding is not needed, and requires including an extra file to access the padding size. RZ ___ 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 v5 1/2] avcodec/avutil: move dynamic HDR10+ metadata parsing to libavutil
On 3/13/23 18:19, James Almer wrote:
> On 3/13/2023 6:38 PM, Raphaël Zumer wrote:
>> diff --git a/libavutil/hdr_dynamic_metadata.c
>> b/libavutil/hdr_dynamic_metadata.c
>> index 0fa1ee82de..98f399b032 100644
>> --- a/libavutil/hdr_dynamic_metadata.c
>> +++ b/libavutil/hdr_dynamic_metadata.c
>> @@ -20,6 +20,16 @@
>>
>> #include "hdr_dynamic_metadata.h"
>> #include "mem.h"
>> +#include "libavcodec/get_bits.h"
>> +#include "libavcodec/put_bits.h"
>> +
>> +static const int64_t luminance_den = 1;
>> +static const int32_t peak_luminance_den = 15;
>> +static const int64_t rgb_den = 10;
>> +static const int32_t fraction_pixel_den = 1000;
>> +static const int32_t knee_point_den = 4095;
>> +static const int32_t bezier_anchor_den = 1023;
>> +static const int32_t saturation_weight_den = 8;
>>
>> AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
>> {
>> @@ -45,3 +55,173 @@ AVDynamicHDRPlus
>> *av_dynamic_hdr_plus_create_side_data(AVFrame *frame)
>>
>> return (AVDynamicHDRPlus *)side_data->data;
>> }
>> +
>> +int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>> + int size)
> I'll change the signature to use size_t instead of int before pushing if
> you don't mind, so it's in line with the serialization function and
> matching the size type of the AVFrame and AVPacket side data structs.
Sure, sounds good.
Thanks
RZ
___
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".
[FFmpeg-devel] [PATCH v5 2/2] avutil: add HDR10+ dynamic metadata serialization function
Co-authored-by: Mohammad Izadi
Signed-off-by: Raphaël Zumer
---
doc/APIchanges | 5 ++
libavutil/hdr_dynamic_metadata.c | 145 +++
libavutil/hdr_dynamic_metadata.h | 13 +++
libavutil/version.h | 2 +-
4 files changed, 164 insertions(+), 1 deletion(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 14737223cb..3a61d61931 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,11 @@ The last version increases of all libraries were on 2023-02-09
API changes, most recent first:
+2023-03-13 - xx - lavu 58.4.100 - hdr_dynamic_metadata.h
+ Add av_dynamic_hdr_plus_from_t35() and av_dynamic_hdr_plus_to_t35()
+ functions to convert between raw T.35 payloads containing dynamic
+ HDR10+ metadata and their parsed representations as AVDynamicHDRPlus.
+
2023-03-02 - xx - lavc 60.6.100 - avcodec.h
Add FF_PROFILE_EAC3_DDP_ATMOS, FF_PROFILE_TRUEHD_ATMOS,
FF_PROFILE_DTS_HD_MA_X and FF_PROFILE_DTS_HD_MA_X_IMAX.
diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 98f399b032..9e35d929c7 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -225,3 +225,148 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s,
const uint8_t *data,
return 0;
}
+
+int av_dynamic_hdr_plus_to_t35(uint8_t **data, size_t *size, const
AVDynamicHDRPlus *s)
+{
+uint8_t *buf;
+size_t size_bits, size_bytes;
+PutBitContext pbc, *pb =
+
+if (!data || !size || !s)
+return AVERROR(EINVAL);
+
+/**
+ * Buffer size per CTA-861-H p.253-254:
+ * 48 bits for the header (56 minus excluded 8-bit country code)
+ * 2 bits for num_windows
+ * 937 bits for window geometry, for each window above 1
+ * 27 bits for targeted_system_display_maximum_luminance
+ * 1-3855 bits for targeted system display peak luminance information
+ * 0-442 bits for intra-window pixel distribution information
+ * 1-3855 bits for mastering display peak luminance information
+ * 0-537 bits for per-window tonemapping information
+ * 0-21 bits for per-window color saturation mapping information
+ */
+size_bits = 48 +
+2 +
+FFMAX((s->num_windows - 1), 0) * 937 +
+27 +
+1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_targeted_system_display_actual_peak_luminance *
+s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0)
+
+s->num_windows * 82;
+
+for (int w = 0; w < s->num_windows; w++)
+size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+
+size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_mastering_display_actual_peak_luminance *
+s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+s->num_windows * 1;
+
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].tone_mapping_flag)
+size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+}
+
+size_bits += s->num_windows * 1;
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].color_saturation_mapping_flag)
+size_bits += 6;
+}
+
+size_bytes = (size_bits + 7) / 8;
+
+buf = av_malloc(size_bytes);
+if (!buf)
+return AVERROR(ENOMEM);
+
+init_put_bits(pb, buf, size_bytes);
+
+// itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
+// itu_t_t35_terminal_provider_code shall be 0x003C
+put_bits(pb, 16, 0x003C);
+// itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+put_bits(pb, 16, 0x0001);
+// application_identifier shall be set to 4
+put_bits(pb, 8, 4);
+// application_mode is set to Application Version 1
+put_bits(pb, 8, 1);
+
+// Payload as per CTA-861-H p.253-254
+put_bits(pb, 2, s->num_windows);
+
+for (int w = 1; w < s->num_windows; w++) {
+put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num /
s->params[w].window_upper_left_corner_x.den);
+put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num /
s->params[w].window_upper_left_corner_y.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num /
s->params[w].window_lower_right_corner_x.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num /
s->params[w].window_lower_right_corner_y.den);
+put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+put_bits(pb, 8, s->params[w].rotation_angle);
+put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+put_bits(pb, 16, s->params[w].semiminor_axis_ext
[FFmpeg-devel] [PATCH v5 1/2] avcodec/avutil: move dynamic HDR10+ metadata parsing to libavutil
Signed-off-by: Raphaël Zumer --- libavcodec/Makefile | 6 +- libavcodec/av1dec.c | 6 +- libavcodec/dynamic_hdr10_plus.c | 198 --- libavcodec/dynamic_hdr10_plus.h | 35 -- libavcodec/h2645_sei.c | 6 +- libavcodec/libdav1d.c| 6 +- libavutil/hdr_dynamic_metadata.c | 180 libavutil/hdr_dynamic_metadata.h | 11 ++ 8 files changed, 203 insertions(+), 245 deletions(-) delete mode 100644 libavcodec/dynamic_hdr10_plus.c delete mode 100644 libavcodec/dynamic_hdr10_plus.h diff --git a/libavcodec/Makefile b/libavcodec/Makefile index abae4909d2..408ecd1e31 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -104,7 +104,7 @@ OBJS-$(CONFIG_H264_SEI)+= h264_sei.o h2645_sei.o OBJS-$(CONFIG_HEVCPARSE) += hevc_parse.o hevc_ps.o hevc_data.o \ h2645data.o h2645_parse.o h2645_vui.o OBJS-$(CONFIG_HEVC_SEI)+= hevc_sei.o h2645_sei.o \ - dynamic_hdr10_plus.o dynamic_hdr_vivid.o + dynamic_hdr_vivid.o OBJS-$(CONFIG_HPELDSP) += hpeldsp.o OBJS-$(CONFIG_HUFFMAN) += huffman.o OBJS-$(CONFIG_HUFFYUVDSP) += huffyuvdsp.o @@ -250,7 +250,7 @@ OBJS-$(CONFIG_ATRAC3PAL_DECODER) += atrac3plusdec.o atrac3plus.o \ OBJS-$(CONFIG_ATRAC9_DECODER) += atrac9dec.o OBJS-$(CONFIG_AURA_DECODER)+= cyuv.o OBJS-$(CONFIG_AURA2_DECODER) += aura.o -OBJS-$(CONFIG_AV1_DECODER) += av1dec.o dynamic_hdr10_plus.o +OBJS-$(CONFIG_AV1_DECODER) += av1dec.o OBJS-$(CONFIG_AV1_CUVID_DECODER) += cuviddec.o OBJS-$(CONFIG_AV1_MEDIACODEC_DECODER) += mediacodecdec.o OBJS-$(CONFIG_AV1_NVENC_ENCODER) += nvenc_av1.o nvenc.o @@ -1082,7 +1082,7 @@ OBJS-$(CONFIG_LIBARIBB24_DECODER) += libaribb24.o ass.o OBJS-$(CONFIG_LIBCELT_DECODER)+= libcelt_dec.o OBJS-$(CONFIG_LIBCODEC2_DECODER) += libcodec2.o OBJS-$(CONFIG_LIBCODEC2_ENCODER) += libcodec2.o -OBJS-$(CONFIG_LIBDAV1D_DECODER) += libdav1d.o dynamic_hdr10_plus.o +OBJS-$(CONFIG_LIBDAV1D_DECODER) += libdav1d.o OBJS-$(CONFIG_LIBDAVS2_DECODER) += libdavs2.o OBJS-$(CONFIG_LIBFDK_AAC_DECODER) += libfdk-aacdec.o OBJS-$(CONFIG_LIBFDK_AAC_ENCODER) += libfdk-aacenc.o diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index a80e37e33f..df393fe3d0 100644 --- a/libavcodec/av1dec.c +++ b/libavcodec/av1dec.c @@ -20,6 +20,7 @@ #include "config_components.h" +#include "libavutil/hdr_dynamic_metadata.h" #include "libavutil/film_grain_params.h" #include "libavutil/mastering_display_metadata.h" #include "libavutil/pixdesc.h" @@ -30,7 +31,6 @@ #include "bytestream.h" #include "codec_internal.h" #include "decode.h" -#include "dynamic_hdr10_plus.h" #include "hwconfig.h" #include "profiles.h" #include "thread.h" @@ -925,8 +925,8 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame, if (!hdrplus) return AVERROR(ENOMEM); -ret = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer, - bytestream2_get_bytes_left()); +ret = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer, + bytestream2_get_bytes_left()); if (ret < 0) return ret; break; diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c deleted file mode 100644 index 34a44aac65..00 --- a/libavcodec/dynamic_hdr10_plus.c +++ /dev/null @@ -1,198 +0,0 @@ -/* - * 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 "dynamic_hdr10_plus.h" -#include "get_bits.h" - -static const int64_t luminance_den = 1; -static const int32_t peak_luminance_den = 15; -static const int64_t rgb_den = 10; -static const int32_t fraction_pixel_den = 1000; -static const int32_t knee_point_den = 4095; -static const int32_t
Re: [FFmpeg-devel] [PATCH v4 2/2] avutil: add HDR10+ dynamic metadata serialization function
On 3/13/23 16:47, James Almer wrote: > On 3/13/2023 5:23 PM, Raphaël Zumer wrote: >> Signed-off-by: Raphaël Zumer >> --- >> doc/APIchanges | 5 ++ >> libavutil/hdr_dynamic_metadata.c | 146 +++ >> libavutil/hdr_dynamic_metadata.h | 12 +++ >> libavutil/version.h | 2 +- >> 4 files changed, 164 insertions(+), 1 deletion(-) > The serialization code here looks almost the same as the one submitted > some time ago by Mohammad Izadi in > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220908012214.1556959-1-iz...@google.com/ > He in fact used the (int)(const AVDynamicHDRPlus* s, uint8_t** data, > size_t* size) signature for the function which does look more versatile > as it allows the return of error codes, fwiw. > > IMO, this patch should either have him as co-author, or a line added to > the commit message mentioning that it's based on his work. I've actually never seen this code, but I agree that this is a better signature. I will fix that and add him as co-author. RZ ___ 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".
[FFmpeg-devel] [PATCH v4 2/2] avutil: add HDR10+ dynamic metadata serialization function
Signed-off-by: Raphaël Zumer
---
doc/APIchanges | 5 ++
libavutil/hdr_dynamic_metadata.c | 146 +++
libavutil/hdr_dynamic_metadata.h | 12 +++
libavutil/version.h | 2 +-
4 files changed, 164 insertions(+), 1 deletion(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 14737223cb..3a61d61931 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,11 @@ The last version increases of all libraries were on 2023-02-09
API changes, most recent first:
+2023-03-13 - xx - lavu 58.4.100 - hdr_dynamic_metadata.h
+ Add av_dynamic_hdr_plus_from_t35() and av_dynamic_hdr_plus_to_t35()
+ functions to convert between raw T.35 payloads containing dynamic
+ HDR10+ metadata and their parsed representations as AVDynamicHDRPlus.
+
2023-03-02 - xx - lavc 60.6.100 - avcodec.h
Add FF_PROFILE_EAC3_DDP_ATMOS, FF_PROFILE_TRUEHD_ATMOS,
FF_PROFILE_DTS_HD_MA_X and FF_PROFILE_DTS_HD_MA_X_IMAX.
diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 98f399b032..0e30c6f246 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -225,3 +225,149 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s,
const uint8_t *data,
return 0;
}
+
+uint8_t *av_dynamic_hdr_plus_to_t35(size_t *size, const AVDynamicHDRPlus *s)
+{
+uint8_t *buf;
+size_t size_bits, size_bytes;
+PutBitContext pbc, *pb =
+
+if (!size)
+return NULL;
+
+if (!s)
+return NULL;
+
+/**
+ * Buffer size per CTA-861-H p.253-254:
+ * 48 bits for the header (56 minus excluded 8-bit country code)
+ * 2 bits for num_windows
+ * 937 bits for window geometry, for each window above 1
+ * 27 bits for targeted_system_display_maximum_luminance
+ * 1-3855 bits for targeted system display peak luminance information
+ * 0-442 bits for intra-window pixel distribution information
+ * 1-3855 bits for mastering display peak luminance information
+ * 0-537 bits for per-window tonemapping information
+ * 0-21 bits for per-window color saturation mapping information
+ */
+size_bits = 48 +
+2 +
+FFMAX((s->num_windows - 1), 0) * 937 +
+27 +
+1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_targeted_system_display_actual_peak_luminance *
+s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0)
+
+s->num_windows * 82;
+
+for (int w = 0; w < s->num_windows; w++)
+size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+
+size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_mastering_display_actual_peak_luminance *
+s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+s->num_windows * 1;
+
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].tone_mapping_flag)
+size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+}
+
+size_bits += s->num_windows * 1;
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].color_saturation_mapping_flag)
+size_bits += 6;
+}
+
+size_bytes = (size_bits + 7) / 8;
+*size = size_bytes;
+
+buf = av_malloc(size_bytes);
+if (!buf)
+return NULL;
+
+init_put_bits(pb, buf, size_bytes);
+
+// itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
+// itu_t_t35_terminal_provider_code shall be 0x003C
+put_bits(pb, 16, 0x003C);
+// itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+put_bits(pb, 16, 0x0001);
+// application_identifier shall be set to 4
+put_bits(pb, 8, 4);
+// application_mode is set to Application Version 1
+put_bits(pb, 8, 1);
+
+// Payload as per CTA-861-H p.253-254
+put_bits(pb, 2, s->num_windows);
+
+for (int w = 1; w < s->num_windows; w++) {
+put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num /
s->params[w].window_upper_left_corner_x.den);
+put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num /
s->params[w].window_upper_left_corner_y.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num /
s->params[w].window_lower_right_corner_x.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num /
s->params[w].window_lower_right_corner_y.den);
+put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+put_bits(pb, 8, s->params[w].rotation_angle);
+put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+put_bits(pb, 16, s->params[w].semiminor_axis_ext
[FFmpeg-devel] [PATCH v4 1/2] avcodec/avutil: move dynamic HDR10+ metadata parsing to libavutil
Signed-off-by: Raphaël Zumer
---
libavcodec/Makefile | 2 +-
libavcodec/dynamic_hdr10_plus.c | 198 ---
libavcodec/dynamic_hdr10_plus.h | 35 --
libavcodec/h2645_sei.c | 6 +-
libavutil/hdr_dynamic_metadata.c | 180
libavutil/hdr_dynamic_metadata.h | 11 ++
6 files changed, 195 insertions(+), 237 deletions(-)
delete mode 100644 libavcodec/dynamic_hdr10_plus.c
delete mode 100644 libavcodec/dynamic_hdr10_plus.h
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index abae4909d2..e8ab55f16e 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -104,7 +104,7 @@ OBJS-$(CONFIG_H264_SEI)+= h264_sei.o
h2645_sei.o
OBJS-$(CONFIG_HEVCPARSE) += hevc_parse.o hevc_ps.o hevc_data.o \
h2645data.o h2645_parse.o h2645_vui.o
OBJS-$(CONFIG_HEVC_SEI)+= hevc_sei.o h2645_sei.o \
- dynamic_hdr10_plus.o
dynamic_hdr_vivid.o
+ dynamic_hdr_vivid.o
OBJS-$(CONFIG_HPELDSP) += hpeldsp.o
OBJS-$(CONFIG_HUFFMAN) += huffman.o
OBJS-$(CONFIG_HUFFYUVDSP) += huffyuvdsp.o
diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
deleted file mode 100644
index 34a44aac65..00
--- a/libavcodec/dynamic_hdr10_plus.c
+++ /dev/null
@@ -1,198 +0,0 @@
-/*
- * 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 "dynamic_hdr10_plus.h"
-#include "get_bits.h"
-
-static const int64_t luminance_den = 1;
-static const int32_t peak_luminance_den = 15;
-static const int64_t rgb_den = 10;
-static const int32_t fraction_pixel_den = 1000;
-static const int32_t knee_point_den = 4095;
-static const int32_t bezier_anchor_den = 1023;
-static const int32_t saturation_weight_den = 8;
-
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const
uint8_t *data,
- int size)
-{
-GetBitContext gbc, *gb =
-int ret;
-
-if (!s)
-return AVERROR(ENOMEM);
-
-ret = init_get_bits8(gb, data, size);
-if (ret < 0)
-return ret;
-
-if (get_bits_left(gb) < 10)
-return AVERROR_INVALIDDATA;
-
-s->application_version = get_bits(gb, 8);
-s->num_windows = get_bits(gb, 2);
-
-if (s->num_windows < 1 || s->num_windows > 3) {
-return AVERROR_INVALIDDATA;
-}
-
-if (get_bits_left(gb) < ((19 * 8 + 1) * (s->num_windows - 1)))
-return AVERROR_INVALIDDATA;
-
-for (int w = 1; w < s->num_windows; w++) {
-// The corners are set to absolute coordinates here. They should be
-// converted to the relative coordinates (in [0, 1]) in the decoder.
-AVHDRPlusColorTransformParams *params = >params[w];
-params->window_upper_left_corner_x =
-(AVRational){get_bits(gb, 16), 1};
-params->window_upper_left_corner_y =
-(AVRational){get_bits(gb, 16), 1};
-params->window_lower_right_corner_x =
-(AVRational){get_bits(gb, 16), 1};
-params->window_lower_right_corner_y =
-(AVRational){get_bits(gb, 16), 1};
-
-params->center_of_ellipse_x = get_bits(gb, 16);
-params->center_of_ellipse_y = get_bits(gb, 16);
-params->rotation_angle = get_bits(gb, 8);
-params->semimajor_axis_internal_ellipse = get_bits(gb, 16);
-params->semimajor_axis_external_ellipse = get_bits(gb, 16);
-params->semiminor_axis_external_ellipse = get_bits(gb, 16);
-params->overlap_process_option = get_bits1(gb);
-}
-
-if (get_bits_left(gb) < 28)
-return AVERROR_INVALIDDATA;
-
-s->targeted_system_display_maximum_luminance =
-(AVRational){get_bits_long(gb, 27), luminance_den};
-s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
-
-if (s->targeted_system_display_actual_peak_luminance_flag) {
-int rows, cols;
-if (get_bits_left(gb) < 10)
-return AVERROR_INVALIDDATA;
-rows = get_bits(gb,
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/avutil: move dynamic HDR metadata parsing to libavutil
On 3/13/23 13:10, James Almer wrote: > On 3/13/2023 2:05 PM, Raphaël Zumer wrote: >> On 3/13/23 12:58, James Almer wrote: >>> On 3/13/2023 1:56 PM, Raphaël Zumer wrote: >>>> On 3/13/23 12:09, Andreas Rheinhardt wrote: >>>>>> >>>>>> +/** >>>>>> + * Parse the user data registered ITU-T T.35 to AVbuffer >>>>>> (AVDynamicHDRVivid). >>>>>> + * @param s A pointer containing the decoded AVDynamicHDRVivid >>>>>> structure. >>>>>> + * @param data The byte array containing the raw ITU-T T.35 data. >>>>>> + * @param size Size of the data array in bytes. >>>>>> + * >>>>>> + * @return 0 if succeed. Otherwise, returns the appropriate AVERROR. >>>>>> + */ >>>>>> +int av_dynamic_hdr_vivid_from_t35(AVDynamicHDRVivid *s, const uint8_t >>>>>> *data, >>>>>> + int size); >>>>> Who has an interest in this function being public? >>>>> >>>>> - Andreas >>>> I have no need for it so can change it to avpriv_ considering there's no >>>> serialization function available for it, if there's no objection to that. >>>> >>>> Raphaël Zumer >>> No, just don't move it out of libavcodec. Unless it's needed elsewhere, >>> it can stay there as is. >> The inconsistency between HDR10+ and Vivid will be confusing IMO if one of >> them is left in libavcodec and the other is moved to libavutil. > Why? The HDR10+ one is useful for libraries like lavf and external > container parsers, the Vivid one isn't. That is obvious in this specific context, but for someone simply looking at the internals and dynamic HDR metadata handling, it would be odd to find a lone function for Vivid HDR parsing sitting in libavcodec while everything else (including the definition for the parsed Vivid metadata structure) is in one place in libavutil. >> What are the specific concerns with making it public (or avpriv), aside from >> it not being useful without a corresponding serialization function? > You said it, it serves no purpose. Making something public (or exposed > internally as avpriv_) is done only when it will be used by code outside > the library where it resides. If it is standard practice in the FFmpeg codebase then I won't argue further, I just think that avpriv accomplishes the desired outcome (keep that function private) without violating best practices (keeping logically-related code together). RZ ___ 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 1/2] avcodec/avutil: move dynamic HDR metadata parsing to libavutil
On 3/13/23 12:58, James Almer wrote: > On 3/13/2023 1:56 PM, Raphaël Zumer wrote: >> On 3/13/23 12:09, Andreas Rheinhardt wrote: >>>> >>>> +/** >>>> + * Parse the user data registered ITU-T T.35 to AVbuffer >>>> (AVDynamicHDRVivid). >>>> + * @param s A pointer containing the decoded AVDynamicHDRVivid structure. >>>> + * @param data The byte array containing the raw ITU-T T.35 data. >>>> + * @param size Size of the data array in bytes. >>>> + * >>>> + * @return 0 if succeed. Otherwise, returns the appropriate AVERROR. >>>> + */ >>>> +int av_dynamic_hdr_vivid_from_t35(AVDynamicHDRVivid *s, const uint8_t >>>> *data, >>>> + int size); >>> Who has an interest in this function being public? >>> >>> - Andreas >> I have no need for it so can change it to avpriv_ considering there's no >> serialization function available for it, if there's no objection to that. >> >> Raphaël Zumer > No, just don't move it out of libavcodec. Unless it's needed elsewhere, > it can stay there as is. The inconsistency between HDR10+ and Vivid will be confusing IMO if one of them is left in libavcodec and the other is moved to libavutil. What are the specific concerns with making it public (or avpriv), aside from it not being useful without a corresponding serialization function? RZ ___ 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 1/2] avcodec/avutil: move dynamic HDR metadata parsing to libavutil
On 3/13/23 12:09, Andreas Rheinhardt wrote: >> >> +/** >> + * Parse the user data registered ITU-T T.35 to AVbuffer >> (AVDynamicHDRVivid). >> + * @param s A pointer containing the decoded AVDynamicHDRVivid structure. >> + * @param data The byte array containing the raw ITU-T T.35 data. >> + * @param size Size of the data array in bytes. >> + * >> + * @return 0 if succeed. Otherwise, returns the appropriate AVERROR. >> + */ >> +int av_dynamic_hdr_vivid_from_t35(AVDynamicHDRVivid *s, const uint8_t *data, >> + int size); > Who has an interest in this function being public? > > - Andreas I have no need for it so can change it to avpriv_ considering there's no serialization function available for it, if there's no objection to that. Raphaël Zumer ___ 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 2/2] avutil: add HDR10+ dynamic metadata serialization function
On 3/12/23 17:52, James Almer wrote: > On 3/12/2023 6:50 PM, Raphaël Zumer wrote: >> I expanded on this in another email in the chain, but the buffer size needs >> to be communicated to the user, as it is not embedded in the payload. It >> seems needlessly convoluted to me to create a separate function solely to >> calculate the size of the buffer so that it can be allocated by the user and >> passed to the serialization function, and I cannot think of another solution >> that would not be even more convoluted and awkward for the user. >> >> I don't understand how going from AVBufferRef to uint8_t* is more >> complicated than the reverse. The buffer in the AVBufferRef is allocated via >> av_malloc() and is directly accessible through the data field. Am I missing >> some detail? >> >> Raphaël Zumer > You can do it like how the AVDynamicHDRPlus struct is allocated and its > size returned to the user in av_dynamic_hdr_plus_alloc(). That is > > uint8_t *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s, size_t *size); > > The function would then store the calculated size of the array on *size. OK great, I will do that and address the remaining comments tomorrow. Thanks >> On 3/12/23 15:48, Anton Khirnov wrote: >>> Quoting Raphaël Zumer (2023-03-02 22:43:29) >>>> +/** >>>> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 >>>> buffer, >>>> + * excluding the country code and beginning with the terminal provider >>>> code. >>>> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure. >>>> + * >>>> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 >>>> representation >>>> + * of the HDR10+ metadata if succeed, or NULL if buffer >>>> allocation fails. >>>> + */ >>>> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s); >>> Why is this an AVBufferRef rather than a plain av_malloced() uint8_t >>> array? You can very easily turn the latter into the former, but the >>> reverse is a lot more annoying. >>> >> ___ >> 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". > ___ > 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". ___ 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 2/2] avutil: add HDR10+ dynamic metadata serialization function
I expanded on this in another email in the chain, but the buffer size needs to be communicated to the user, as it is not embedded in the payload. It seems needlessly convoluted to me to create a separate function solely to calculate the size of the buffer so that it can be allocated by the user and passed to the serialization function, and I cannot think of another solution that would not be even more convoluted and awkward for the user. I don't understand how going from AVBufferRef to uint8_t* is more complicated than the reverse. The buffer in the AVBufferRef is allocated via av_malloc() and is directly accessible through the data field. Am I missing some detail? Raphaël Zumer On 3/12/23 15:48, Anton Khirnov wrote: > Quoting Raphaël Zumer (2023-03-02 22:43:29) >> +/** >> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 >> buffer, >> + * excluding the country code and beginning with the terminal provider code. >> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure. >> + * >> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 >> representation >> + * of the HDR10+ metadata if succeed, or NULL if buffer allocation >> fails. >> + */ >> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s); > Why is this an AVBufferRef rather than a plain av_malloced() uint8_t > array? You can very easily turn the latter into the former, but the > reverse is a lot more annoying. > ___ 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 2/2] avutil: add HDR10+ dynamic metadata serialization function
Hi,
While I omitted adding v2/v3 here, I believe all comments on this set of
patches have been addressed so far, unless anyone strongly disagrees with the
rationale for moving dynamic HDR parsing and serialization to libavutil or with
the function signature.
Please let me know if I missed anything.
Thanks,
Raphaël Zumer
On 3/2/23 16:43, Raphaël Zumer wrote:
> Fixed brace style and moved inline buffer size calculation comments to a
> single block at the top.
>
>
> Signed-off-by: Raphaël Zumer
> ---
> libavutil/hdr_dynamic_metadata.c | 142 +++
> libavutil/hdr_dynamic_metadata.h | 11 +++
> libavutil/version.h | 2 +-
> 3 files changed, 154 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/hdr_dynamic_metadata.c
> b/libavutil/hdr_dynamic_metadata.c
> index 98f399b032..24dd3dab2d 100644
> --- a/libavutil/hdr_dynamic_metadata.c
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -225,3 +225,145 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s,
> const uint8_t *data,
>
> return 0;
> }
> +
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
> +{
> +AVBufferRef *buf;
> +size_t size_bits, size_bytes;
> +PutBitContext pbc, *pb =
> +
> +if (!s)
> +return NULL;
> +
> +/**
> + * Buffer size per CTA-861-H p.253-254:
> + * 48 bits for the header (56 minus excluded 8-bit country code)
> + * 2 bits for num_windows
> + * 937 bits for window geometry, for each window above 1
> + * 27 bits for targeted_system_display_maximum_luminance
> + * 1-3855 bits for targeted system display peak luminance information
> + * 0-442 bits for intra-window pixel distribution information
> + * 1-3855 bits for mastering display peak luminance information
> + * 0-537 bits for per-window tonemapping information
> + * 0-21 bits for per-window color saturation mapping information
> + */
> +size_bits = 48 +
> +2 +
> +FFMAX((s->num_windows - 1), 0) * 937 +
> +27 +
> +1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
> +s->num_rows_targeted_system_display_actual_peak_luminance *
> +s->num_cols_targeted_system_display_actual_peak_luminance * 4 :
> 0) +
> +s->num_windows * 82;
> +
> +for (int w = 0; w < s->num_windows; w++)
> +size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
> +
> +size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
> +s->num_rows_mastering_display_actual_peak_luminance *
> +s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
> +s->num_windows * 1;
> +
> +for (int w = 0; w < s->num_windows; w++) {
> +if (s->params[w].tone_mapping_flag)
> +size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
> +}
> +
> +size_bits += s->num_windows * 1;
> +for (int w = 0; w < s->num_windows; w++) {
> +if (s->params[w].color_saturation_mapping_flag)
> +size_bits += 6;
> +}
> +
> +size_bytes = (size_bits + 7) / 8;
> +
> +buf = av_buffer_alloc(size_bytes);
> +if (!buf)
> +return NULL;
> +
> +init_put_bits(pb, buf->data, size_bytes);
> +
> +// itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
> +// itu_t_t35_terminal_provider_code shall be 0x003C
> +put_bits(pb, 16, 0x003C);
> +// itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
> +put_bits(pb, 16, 0x0001);
> +// application_identifier shall be set to 4
> +put_bits(pb, 8, 4);
> +// application_mode is set to Application Version 1
> +put_bits(pb, 8, 1);
> +
> +// Payload as per CTA-861-H p.253-254
> +put_bits(pb, 2, s->num_windows);
> +
> +for (int w = 1; w < s->num_windows; w++) {
> +put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num /
> s->params[w].window_upper_left_corner_x.den);
> +put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num /
> s->params[w].window_upper_left_corner_y.den);
> +put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num /
> s->params[w].window_lower_right_corner_x.den);
> +put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num /
> s->params[w].window_lower_right_corner_y.den);
> +put_bits(pb, 16, s->params[w].center_of_ellipse_x);
> +put_bits(pb, 16, s->params[w].center_of_ellipse_y);
> +put_bits(pb, 8, s->params[w].rotation_angle);
> +put_bits(pb, 16, s->
[FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata serialization function
Fixed brace style and moved inline buffer size calculation comments to a single
block at the top.
Signed-off-by: Raphaël Zumer
---
libavutil/hdr_dynamic_metadata.c | 142 +++
libavutil/hdr_dynamic_metadata.h | 11 +++
libavutil/version.h | 2 +-
3 files changed, 154 insertions(+), 1 deletion(-)
diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 98f399b032..24dd3dab2d 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -225,3 +225,145 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s,
const uint8_t *data,
return 0;
}
+
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
+{
+AVBufferRef *buf;
+size_t size_bits, size_bytes;
+PutBitContext pbc, *pb =
+
+if (!s)
+return NULL;
+
+/**
+ * Buffer size per CTA-861-H p.253-254:
+ * 48 bits for the header (56 minus excluded 8-bit country code)
+ * 2 bits for num_windows
+ * 937 bits for window geometry, for each window above 1
+ * 27 bits for targeted_system_display_maximum_luminance
+ * 1-3855 bits for targeted system display peak luminance information
+ * 0-442 bits for intra-window pixel distribution information
+ * 1-3855 bits for mastering display peak luminance information
+ * 0-537 bits for per-window tonemapping information
+ * 0-21 bits for per-window color saturation mapping information
+ */
+size_bits = 48 +
+2 +
+FFMAX((s->num_windows - 1), 0) * 937 +
+27 +
+1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_targeted_system_display_actual_peak_luminance *
+s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0)
+
+s->num_windows * 82;
+
+for (int w = 0; w < s->num_windows; w++)
+size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+
+size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_mastering_display_actual_peak_luminance *
+s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+s->num_windows * 1;
+
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].tone_mapping_flag)
+size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+}
+
+size_bits += s->num_windows * 1;
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].color_saturation_mapping_flag)
+size_bits += 6;
+}
+
+size_bytes = (size_bits + 7) / 8;
+
+buf = av_buffer_alloc(size_bytes);
+if (!buf)
+return NULL;
+
+init_put_bits(pb, buf->data, size_bytes);
+
+// itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
+// itu_t_t35_terminal_provider_code shall be 0x003C
+put_bits(pb, 16, 0x003C);
+// itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+put_bits(pb, 16, 0x0001);
+// application_identifier shall be set to 4
+put_bits(pb, 8, 4);
+// application_mode is set to Application Version 1
+put_bits(pb, 8, 1);
+
+// Payload as per CTA-861-H p.253-254
+put_bits(pb, 2, s->num_windows);
+
+for (int w = 1; w < s->num_windows; w++) {
+put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num /
s->params[w].window_upper_left_corner_x.den);
+put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num /
s->params[w].window_upper_left_corner_y.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num /
s->params[w].window_lower_right_corner_x.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num /
s->params[w].window_lower_right_corner_y.den);
+put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+put_bits(pb, 8, s->params[w].rotation_angle);
+put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
+put_bits(pb, 1, s->params[w].overlap_process_option);
+}
+
+put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num *
luminance_den /
+s->targeted_system_display_maximum_luminance.den);
+put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
+if (s->targeted_system_display_actual_peak_luminance_flag) {
+put_bits(pb, 5,
s->num_rows_targeted_system_display_actual_peak_luminance);
+put_bits(pb, 5,
s->num_cols_targeted_system_display_actual_peak_luminance);
+for (int i = 0; i <
s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
+for (int j = 0; j <
s->num_co
Re: [FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata serialization function
On 3/2/23 15:24, Leo Izen wrote:
> On 3/2/23 14:25, Raphaël Zumer wrote:
>> Signed-off-by: Raphaël Zumer
>> ---
>> libavutil/hdr_dynamic_metadata.c | 146 +++
>> libavutil/hdr_dynamic_metadata.h | 11 +++
>> libavutil/version.h | 2 +-
>> 3 files changed, 158 insertions(+), 1 deletion(-)
>>
> Why not put this in avcodec/dynamic_hdr10_plus.c? You reference
> put_bits.h which is in avcodec, so that can possibly be an issue, even
> though it is inlined (i.e. it sends the wrong message since avutil is
> supposed to not depend on avcodec).
I agree it is somewhat awkward to introduce a circular dependency (albeit to
header-only files). On the other hand, I think those functions make more sense
in libavutil than libavcodec, and it improves readability by not splitting
files that are logically connected between libraries. If there is a general
consensus that it is better to keep them in libavcodec, I don't mind reverting
that change, or moving get_bits and put_bits to libavutil if that is doable.
>> diff --git a/libavutil/hdr_dynamic_metadata.c
>> b/libavutil/hdr_dynamic_metadata.c
>> index 98f399b032..39a7886a2e 100644
>> --- a/libavutil/hdr_dynamic_metadata.c
>> +++ b/libavutil/hdr_dynamic_metadata.c
>> @@ -225,3 +225,149 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s,
>> const uint8_t *data,
>>
>> return 0;
>> }
>> +
>> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
>> +{
> av_dynamic_hdr_plus_from_t35 returns an int status code and takes a
> pointer as an argument, is there any particular reason you didn't mirror
> user interface here?
Mainly the added complexity of buffer size calculation. I think it would be
doable by adding an additional function such as
av_dynamic_hdr_plus_to_t35_size() that would return the serialized buffer size,
which could be then used by the user to allocate a buffer to be written by
av_dynamic_hdr_plus_to_t35(). But adding an additional function just to make
the function signatures consistent feels contrived to me, and there aren't
several errors that could happen in that function that would need to be
disambiguated by the user.
>> +AVBufferRef *buf;
>> +size_t size_bits, size_bytes;
>> +PutBitContext pbc, *pb =
>> +
>> +if (!s)
>> +return NULL;
>> +
>> +// Buffer size per CTA-861-H p.253-254:
>> +size_bits =
>> +// 56 bits for the header, minus 8-bit excluded country code
>> +48 +
>> +// 2 bits for num_windows
>> +2 +
>> +// 937 bits for window geometry for each window above 1
>> +FFMAX((s->num_windows - 1), 0) * 937 +
>> +// 27 bits for targeted_system_display_maximum_luminance
>> +27 +
>> +// 1-3855 bits for targeted system display peak luminance information
>> +1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
>> +s->num_rows_targeted_system_display_actual_peak_luminance *
>> +s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
>> +// 0-442 bits for intra-window pixel distribution information
>> +s->num_windows * 82;
> This sequence above is difficult to read due to the inline // comments.
> It should be more readable to just have the entire expression be
> contiguous with a /* */ multiline block comment above it explaining each
> item.
>> +for (int w = 0; w < s->num_windows; w++) {
>> +size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
>> +}
> Likewise, another code style issue, don't use {} to enclose a single
> line unless it's unavoidable. This occurs in several places in this patch.
OK, will correct.
Thanks
Raphaël Zumer
___
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".
[FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata serialization function
Signed-off-by: Raphaël Zumer
---
libavutil/hdr_dynamic_metadata.c | 146 +++
libavutil/hdr_dynamic_metadata.h | 11 +++
libavutil/version.h | 2 +-
3 files changed, 158 insertions(+), 1 deletion(-)
diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 98f399b032..39a7886a2e 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -225,3 +225,149 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s,
const uint8_t *data,
return 0;
}
+
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
+{
+AVBufferRef *buf;
+size_t size_bits, size_bytes;
+PutBitContext pbc, *pb =
+
+if (!s)
+return NULL;
+
+// Buffer size per CTA-861-H p.253-254:
+size_bits =
+// 56 bits for the header, minus 8-bit excluded country code
+48 +
+// 2 bits for num_windows
+2 +
+// 937 bits for window geometry for each window above 1
+FFMAX((s->num_windows - 1), 0) * 937 +
+// 27 bits for targeted_system_display_maximum_luminance
+27 +
+// 1-3855 bits for targeted system display peak luminance information
+1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_targeted_system_display_actual_peak_luminance *
+s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
+// 0-442 bits for intra-window pixel distribution information
+s->num_windows * 82;
+for (int w = 0; w < s->num_windows; w++) {
+size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+}
+// 1-3855 bits for mastering display peak luminance information
+size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_mastering_display_actual_peak_luminance *
+s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+// 0-537 bits for per-window tonemapping information
+s->num_windows * 1;
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].tone_mapping_flag) {
+size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+}
+}
+// 0-21 bits for per-window color saturation mapping information
+size_bits += s->num_windows * 1;
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].color_saturation_mapping_flag) {
+size_bits += 6;
+}
+}
+
+size_bytes = (size_bits + 7) / 8;
+
+buf = av_buffer_alloc(size_bytes);
+if (!buf) {
+return NULL;
+}
+
+init_put_bits(pb, buf->data, size_bytes);
+
+// itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
+// itu_t_t35_terminal_provider_code shall be 0x003C
+put_bits(pb, 16, 0x003C);
+// itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+put_bits(pb, 16, 0x0001);
+// application_identifier shall be set to 4
+put_bits(pb, 8, 4);
+// application_mode is set to Application Version 1
+put_bits(pb, 8, 1);
+
+// Payload as per CTA-861-H p.253-254
+put_bits(pb, 2, s->num_windows);
+
+for (int w = 1; w < s->num_windows; w++) {
+put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num /
s->params[w].window_upper_left_corner_x.den);
+put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num /
s->params[w].window_upper_left_corner_y.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num /
s->params[w].window_lower_right_corner_x.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num /
s->params[w].window_lower_right_corner_y.den);
+put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+put_bits(pb, 8, s->params[w].rotation_angle);
+put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
+put_bits(pb, 1, s->params[w].overlap_process_option);
+}
+
+put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num *
luminance_den /
+s->targeted_system_display_maximum_luminance.den);
+put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
+if (s->targeted_system_display_actual_peak_luminance_flag) {
+put_bits(pb, 5,
s->num_rows_targeted_system_display_actual_peak_luminance);
+put_bits(pb, 5,
s->num_cols_targeted_system_display_actual_peak_luminance);
+for (int i = 0; i <
s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
+for (int j = 0; j <
s->num_cols_targeted_system_display_actual_peak_luminance; j++) {
+put_bits(pb, 4,
s->targeted_s
Re: [FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata serialization function
On 3/2/23 13:57, James Almer wrote: > > The SEI for HEVC does keep the country code in the payload, but not AV1. > Are you sure about HEVC? I just checked a sample and trace_headers > reported this: > > [trace_headers] Prefix Supplemental Enhancement Information > [trace_headers] 0 forbidden_zero_bit 0 = 0 > [trace_headers] 1 nal_unit_type 100111 = 39 > [trace_headers] 7 nuh_layer_id00 = 0 > [trace_headers] 13 nuh_temporal_id_plus1 001 = 1 > [trace_headers] 16 last_payload_type_byte0100 = 4 > [trace_headers] 24 last_payload_size_byte0100 = 64 > [trace_headers] User Data Registered ITU-T T.35 > [trace_headers] 32 itu_t_t35_country_code10110101 = 181 > [trace_headers] 40 itu_t_t35_payload_byte[1] = 0 > [trace_headers] 48 itu_t_t35_payload_byte[2] 0000 = 60 > [trace_headers] 56 itu_t_t35_payload_byte[3] = 0 > [trace_headers] 64 itu_t_t35_payload_byte[4] 0001 = 1 > [trace_headers] 72 itu_t_t35_payload_byte[5] 0100 = 4 > [trace_headers] 80 itu_t_t35_payload_byte[6] 0001 = 1 > > Which is in line with section 8.3.1 of ITU-T Rec. H.274 as well as > section D.2.6 of ITU-T Rec. H.265. > > So i think this function should not set the country code at all as part > of the payload, and start with itu_t_t35_terminal_provider_code. OK, I will make that change since both HEVC and AV1 implementations seem to match. Thanks Raphaël Zumer ___ 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 2/2] avutil: add HDR10+ dynamic metadata serialization function
On 3/2/23 13:33, quietvoid wrote: > This patch series mentions that it would be used for AV1, however the AV1 > specification is clear that the payload does not contain the country code. > https://aomediacodec.github.io/av1-hdr10plus/#hdr10plus-metadata, Figure 1. > > So far all the AV1 HDR10+ conformance samples also respect this. > There would probably be a need to specify which behaviour is wanted. > The SEI for HEVC does keep the country code in the payload, but not AV1. As you pointed out, HEVC does include the country code in the payload, so either approach can be seen as reasonable. There are a few reasons why I don't think it makes sense to reproduce the AV1 specification: - it does not make sense to exclude the country code from the payload while including other static headers like the terminal provider codes and the application identifier, so there is no particular advantage or logic that I can see in splitting the payload in that way. - pragmatically, it is much less inconvenient to deal with a 1-byte offset in the payload for AV1 than it is to insert that byte into the payload for HEVC and other applications that expect the country code to be included in the header. For the same reason, I think that an option to include or exclude the country code byte would be more trouble than it's worth. - perhaps most importantly, the payload syntax is standardized in ANSI/CTA-861-H, which does not make any distinction between the header (country code, terminal provider code, etc.) and the remainder of the payload, so the AV1 specification does not conform either to other implementations nor to the standard. Following the most authoritative document is the safest route in my opinion. Raphaël Zumer ___ 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".
[FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata serialization function
Resent due to my mail client incorrectly re-wrapping lines in the version I
sent earlier.
See the previous patch for context.
Signed-off-by: Raphaël Zumer
---
libavutil/hdr_dynamic_metadata.c | 147 +++
libavutil/hdr_dynamic_metadata.h | 10 +++
libavutil/version.h | 2 +-
3 files changed, 158 insertions(+), 1 deletion(-)
diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 98f399b032..72d310e633 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -225,3 +225,150 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s,
const uint8_t *data,
return 0;
}
+
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
+{
+AVBufferRef *buf;
+size_t size_bits, size_bytes;
+PutBitContext pbc, *pb =
+
+if (!s)
+return NULL;
+
+// Buffer size per CTA-861-H p.253-254:
+size_bits =
+// 56 bits for the header
+58 +
+// 2 bits for num_windows
+2 +
+// 937 bits for window geometry for each window above 1
+FFMAX((s->num_windows - 1), 0) * 937 +
+// 27 bits for targeted_system_display_maximum_luminance
+27 +
+// 1-3855 bits for targeted system display peak luminance information
+1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_targeted_system_display_actual_peak_luminance *
+s->num_cols_targeted_system_display_actual_peak_luminance * 4 : 0) +
+// 0-442 bits for intra-window pixel distribution information
+s->num_windows * 82;
+for (int w = 0; w < s->num_windows; w++) {
+size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+}
+// 1-3855 bits for mastering display peak luminance information
+size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_mastering_display_actual_peak_luminance *
+s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+// 0-537 bits for per-window tonemapping information
+s->num_windows * 1;
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].tone_mapping_flag) {
+size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+}
+}
+// 0-21 bits for per-window color saturation mapping information
+size_bits += s->num_windows * 1;
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].color_saturation_mapping_flag) {
+size_bits += 6;
+}
+}
+
+size_bytes = (size_bits + 7) / 8;
+
+buf = av_buffer_alloc(size_bytes);
+if (!buf) {
+return NULL;
+}
+
+init_put_bits(pb, buf->data, size_bytes);
+
+// itu_t_t35_country_code shall be 0xB5 (USA)
+put_bits(pb, 8, 0xB5);
+// itu_t_t35_terminal_provider_code shall be 0x003C
+put_bits(pb, 16, 0x003C);
+// itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+put_bits(pb, 16, 0x0001);
+// application_identifier shall be set to 4
+put_bits(pb, 8, 4);
+// application_mode is set to Application Version 1
+put_bits(pb, 8, 1);
+
+// Payload as per CTA-861-H p.253-254
+put_bits(pb, 2, s->num_windows);
+
+for (int w = 1; w < s->num_windows; w++) {
+put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num /
s->params[w].window_upper_left_corner_x.den);
+put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num /
s->params[w].window_upper_left_corner_y.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num /
s->params[w].window_lower_right_corner_x.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num /
s->params[w].window_lower_right_corner_y.den);
+put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+put_bits(pb, 8, s->params[w].rotation_angle);
+put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
+put_bits(pb, 1, s->params[w].overlap_process_option);
+}
+
+put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num *
luminance_den /
+s->targeted_system_display_maximum_luminance.den);
+put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
+if (s->targeted_system_display_actual_peak_luminance_flag) {
+put_bits(pb, 5,
s->num_rows_targeted_system_display_actual_peak_luminance);
+put_bits(pb, 5,
s->num_cols_targeted_system_display_actual_peak_luminance);
+for (int i = 0; i <
s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
+for (int j = 0; j <
s->num_cols_targeted_system_disp
[FFmpeg-devel] [PATCH 1/2] avcodec/avutil: move dynamic HDR metadata parsing to libavutil
Resending this patch set due to my mail client messing with the line wrapping
in the messages I sent earlier today.
Below is a copy of the initial explanation.
This patch set implements serialization for HDR10+ dynamic metadata
(AVDynamicHDRPlus), which is the inverse operation of the existing
ff_parse_itu_t_t35_to_dynamic_hdr10_plus() function. It also moves both
functions from libavcodec to libavutil and makes them public. For
consistency, the equivalent vivid HDR parsing function is also migrated,
but I did not implement serialization for it. Finally, the patch renames
those functions to av_dynamic_hdr_plus_from_t35() (for parsing) and
av_dynamic_hdr_plus_to_t35 (for serialization), with the equivalent
change being made for vivid as well.
The motivation for this change is to allow users to easily convert
HDR10+ side data (which is parsed into AVDynamicHDRPlus) to a standard
ITU-T T.35 payload that can be passed directly to applications that
expect HDR10+ dynamic metadata in that format (e.g. x265 and rav1e
encoders).
The return value of the serialization function is AVBufferRef*, which I
expect to be contentious. Payload size is not embedded in the T.35 data,
so it must be calculated, used to allocate a buffer, and returned along
with that buffer to the user. As far as I'm aware, AVBufferRef is the
simplest way to do that, but I will be happy to consider alternative
solutions.
Please let me know if it is preferred to bump libavutil with the first
commit, or with both of them, considering there are public API changes
associated with each one.
Raphaël Zumer
Signed-off-by: Raphaël Zumer
---
libavcodec/Makefile| 3 +-
libavcodec/dynamic_hdr10_plus.c| 198 -
libavcodec/dynamic_hdr10_plus.h| 35 -
libavcodec/dynamic_hdr_vivid.c | 139 -
libavcodec/dynamic_hdr_vivid.h | 35 -
libavcodec/h2645_sei.c | 12 +-
libavutil/hdr_dynamic_metadata.c | 180 ++
libavutil/hdr_dynamic_metadata.h | 11 ++
libavutil/hdr_dynamic_vivid_metadata.c | 120 +++
libavutil/hdr_dynamic_vivid_metadata.h | 11 ++
10 files changed, 329 insertions(+), 415 deletions(-)
delete mode 100644 libavcodec/dynamic_hdr10_plus.c
delete mode 100644 libavcodec/dynamic_hdr10_plus.h
delete mode 100644 libavcodec/dynamic_hdr_vivid.c
delete mode 100644 libavcodec/dynamic_hdr_vivid.h
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 389253f5d0..4bdfcbab12 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -103,8 +103,7 @@ OBJS-$(CONFIG_H264QPEL)+= h264qpel.o
OBJS-$(CONFIG_H264_SEI)+= h264_sei.o h2645_sei.o
OBJS-$(CONFIG_HEVCPARSE) += hevc_parse.o hevc_ps.o hevc_data.o \
h2645data.o h2645_parse.o h2645_vui.o
-OBJS-$(CONFIG_HEVC_SEI)+= hevc_sei.o h2645_sei.o \
- dynamic_hdr10_plus.o
dynamic_hdr_vivid.o
+OBJS-$(CONFIG_HEVC_SEI)+= hevc_sei.o h2645_sei.o
OBJS-$(CONFIG_HPELDSP) += hpeldsp.o
OBJS-$(CONFIG_HUFFMAN) += huffman.o
OBJS-$(CONFIG_HUFFYUVDSP) += huffyuvdsp.o
diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
deleted file mode 100644
index 34a44aac65..00
--- a/libavcodec/dynamic_hdr10_plus.c
+++ /dev/null
@@ -1,198 +0,0 @@
-/*
- * 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 "dynamic_hdr10_plus.h"
-#include "get_bits.h"
-
-static const int64_t luminance_den = 1;
-static const int32_t peak_luminance_den = 15;
-static const int64_t rgb_den = 10;
-static const int32_t fraction_pixel_den = 1000;
-static const int32_t knee_point_den = 4095;
-static const int32_t bezier_anchor_den = 1023;
-static const int32_t saturation_weight_den = 8;
-
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const
uint8_t *data,
- int size)
-{
-GetBitContext gbc, *gb =
-int ret;
-
-if (!s)
-return AVERROR(ENOMEM);
-
-ret = init_get_bits8(gb, data, size);
-if (ret <
[FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata serialization function
See the previous patch for context.
Signed-off-by: Raphaël Zumer
---
libavutil/hdr_dynamic_metadata.c | 147 +++
libavutil/hdr_dynamic_metadata.h | 10 +++
libavutil/version.h | 2 +-
3 files changed, 158 insertions(+), 1 deletion(-)
diff --git a/libavutil/hdr_dynamic_metadata.c
b/libavutil/hdr_dynamic_metadata.c
index 98f399b032..72d310e633 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -225,3 +225,150 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus
*s, const uint8_t *data,
return 0;
}
+
+AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
+{
+AVBufferRef *buf;
+size_t size_bits, size_bytes;
+PutBitContext pbc, *pb =
+
+if (!s)
+return NULL;
+
+// Buffer size per CTA-861-H p.253-254:
+size_bits =
+// 56 bits for the header
+58 +
+// 2 bits for num_windows
+2 +
+// 937 bits for window geometry for each window above 1
+FFMAX((s->num_windows - 1), 0) * 937 +
+// 27 bits for targeted_system_display_maximum_luminance
+27 +
+// 1-3855 bits for targeted system display peak luminance information
+1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
+s->num_rows_targeted_system_display_actual_peak_luminance *
+s->num_cols_targeted_system_display_actual_peak_luminance * 4 :
0) +
+// 0-442 bits for intra-window pixel distribution information
+s->num_windows * 82;
+for (int w = 0; w < s->num_windows; w++) {
+size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
+}
+// 1-3855 bits for mastering display peak luminance information
+size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ?
10 +
+s->num_rows_mastering_display_actual_peak_luminance *
+s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
+// 0-537 bits for per-window tonemapping information
+s->num_windows * 1;
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].tone_mapping_flag) {
+size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
+}
+}
+// 0-21 bits for per-window color saturation mapping information
+size_bits += s->num_windows * 1;
+for (int w = 0; w < s->num_windows; w++) {
+if (s->params[w].color_saturation_mapping_flag) {
+size_bits += 6;
+}
+}
+
+size_bytes = (size_bits + 7) / 8;
+
+buf = av_buffer_alloc(size_bytes);
+if (!buf) {
+return NULL;
+}
+
+init_put_bits(pb, buf->data, size_bytes);
+
+// itu_t_t35_country_code shall be 0xB5 (USA)
+put_bits(pb, 8, 0xB5);
+// itu_t_t35_terminal_provider_code shall be 0x003C
+put_bits(pb, 16, 0x003C);
+// itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
+put_bits(pb, 16, 0x0001);
+// application_identifier shall be set to 4
+put_bits(pb, 8, 4);
+// application_mode is set to Application Version 1
+put_bits(pb, 8, 1);
+
+// Payload as per CTA-861-H p.253-254
+put_bits(pb, 2, s->num_windows);
+
+for (int w = 1; w < s->num_windows; w++) {
+put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num /
s->params[w].window_upper_left_corner_x.den);
+put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num /
s->params[w].window_upper_left_corner_y.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num /
s->params[w].window_lower_right_corner_x.den);
+put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num /
s->params[w].window_lower_right_corner_y.den);
+put_bits(pb, 16, s->params[w].center_of_ellipse_x);
+put_bits(pb, 16, s->params[w].center_of_ellipse_y);
+put_bits(pb, 8, s->params[w].rotation_angle);
+put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
+put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
+put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
+put_bits(pb, 1, s->params[w].overlap_process_option);
+}
+
+put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num *
luminance_den /
+s->targeted_system_display_maximum_luminance.den);
+put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
+if (s->targeted_system_display_actual_peak_luminance_flag) {
+put_bits(pb, 5,
s->num_rows_targeted_system_display_actual_peak_luminance);
+put_bits(pb, 5,
s->num_cols_targeted_system_display_actual_peak_luminance);
+for (int i = 0; i <
s->num_rows_targeted_system_display_actual_peak_luminance; i++) {
+for (int j = 0; j <
s->num_cols_targeted_system_display_actual_peak_luminance; j++) {
+put_bits(pb, 4
[FFmpeg-devel] [PATCH 1/2] avcodec/avutil: move dynamic HDR metadata parsing to libavutil
This patch set implements serialization for HDR10+ dynamic metadata
(AVDynamicHDRPlus), which is the inverse operation of the existing
ff_parse_itu_t_t35_to_dynamic_hdr10_plus() function. It also moves both
functions from libavcodec to libavutil and makes them public. For
consistency, the equivalent vivid HDR parsing function is also migrated,
but I did not implement serialization for it. Finally, the patch renames
those functions to av_dynamic_hdr_plus_from_t35() (for parsing) and
av_dynamic_hdr_plus_to_t35 (for serialization), with the equivalent
change being made for vivid as well.
The motivation for this change is to allow users to easily convert
HDR10+ side data (which is parsed into AVDynamicHDRPlus) to a standard
ITU-T T.35 payload that can be passed directly to applications that
expect HDR10+ dynamic metadata in that format (e.g. x265 and rav1e
encoders).
The return value of the serialization function is AVBufferRef*, which I
expect to be contentious. Payload size is not embedded in the T.35 data,
so it must be calculated, used to allocate a buffer, and returned along
with that buffer to the user. As far as I'm aware, AVBufferRef is the
simplest way to do that, but I will be happy to consider alternative
solutions.
Please let me know if it is preferred to bump libavutil with the first
commit, or with both of them, considering there are public API changes
associated with each one.
Raphaël Zumer
Signed-off-by: Raphaël Zumer
---
libavcodec/Makefile| 3 +-
libavcodec/dynamic_hdr10_plus.c| 198 -
libavcodec/dynamic_hdr10_plus.h| 35 -
libavcodec/dynamic_hdr_vivid.c | 139 -
libavcodec/dynamic_hdr_vivid.h | 35 -
libavcodec/h2645_sei.c | 12 +-
libavutil/hdr_dynamic_metadata.c | 180 ++
libavutil/hdr_dynamic_metadata.h | 11 ++
libavutil/hdr_dynamic_vivid_metadata.c | 120 +++
libavutil/hdr_dynamic_vivid_metadata.h | 11 ++
10 files changed, 329 insertions(+), 415 deletions(-)
delete mode 100644 libavcodec/dynamic_hdr10_plus.c
delete mode 100644 libavcodec/dynamic_hdr10_plus.h
delete mode 100644 libavcodec/dynamic_hdr_vivid.c
delete mode 100644 libavcodec/dynamic_hdr_vivid.h
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 389253f5d0..4bdfcbab12 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -103,8 +103,7 @@ OBJS-$(CONFIG_H264QPEL)+= h264qpel.o
OBJS-$(CONFIG_H264_SEI)+= h264_sei.o h2645_sei.o
OBJS-$(CONFIG_HEVCPARSE) += hevc_parse.o hevc_ps.o
hevc_data.o \
h2645data.o h2645_parse.o
h2645_vui.o
-OBJS-$(CONFIG_HEVC_SEI)+= hevc_sei.o h2645_sei.o \
- dynamic_hdr10_plus.o
dynamic_hdr_vivid.o
+OBJS-$(CONFIG_HEVC_SEI)+= hevc_sei.o h2645_sei.o
OBJS-$(CONFIG_HPELDSP) += hpeldsp.o
OBJS-$(CONFIG_HUFFMAN) += huffman.o
OBJS-$(CONFIG_HUFFYUVDSP) += huffyuvdsp.o
diff --git a/libavcodec/dynamic_hdr10_plus.c
b/libavcodec/dynamic_hdr10_plus.c
deleted file mode 100644
index 34a44aac65..00
--- a/libavcodec/dynamic_hdr10_plus.c
+++ /dev/null
@@ -1,198 +0,0 @@
-/*
- * 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 "dynamic_hdr10_plus.h"
-#include "get_bits.h"
-
-static const int64_t luminance_den = 1;
-static const int32_t peak_luminance_den = 15;
-static const int64_t rgb_den = 10;
-static const int32_t fraction_pixel_den = 1000;
-static const int32_t knee_point_den = 4095;
-static const int32_t bezier_anchor_den = 1023;
-static const int32_t saturation_weight_den = 8;
-
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const
uint8_t *data,
- int size)
-{
-GetBitContext gbc, *gb =
-int ret;
-
-if (!s)
-return AVERROR(ENOMEM);
-
-ret = init_get_bits8(gb, data, size);
-if (ret < 0)
-return ret;
-
-if (get_bits_left(gb) < 10)
-return AVERROR_INVALIDDATA;
-
-s->application_version = get_bits(gb, 8);
-s-
Re: [FFmpeg-devel] [PATCH] avformat/ivfenc: Change the length fields to 32 bits
Just sending a reminder for my set of patches (the set of v2 patches up in the thread and this one). Are there any further comments? Thanks Raphaël Zumer On Wed, 2019-10-02 at 09:04 -0400, Raphaël Zumer wrote: > There is no change in the encoded bitstream, but this > ensures that the written field length is consistent > with the reference implementation. > > Unused bytes are zeroed out for backwards compatibility. > > Signed-off-by: Raphaël Zumer > --- > libavformat/ivfenc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c > index ae461a872b..eb70421c44 100644 > --- a/libavformat/ivfenc.c > +++ b/libavformat/ivfenc.c > @@ -84,7 +84,8 @@ static int ivf_write_trailer(AVFormatContext *s) > > avio_seek(pb, 24, SEEK_SET); > // overwrite the "length" field (duration) > -avio_wl64(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx- > >frame_cnt - 1)); > +avio_wl32(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx- > >frame_cnt - 1)); > +avio_wl32(pb, 0); // zero out unused bytes > avio_seek(pb, end, SEEK_SET); > } > ___ 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] avformat/ivf: Change the length field to 32 bits
On Tue, 2019-10-01 at 15:57 -0400, Calvin Walton wrote: > On Tue, 2019-10-01 at 21:41 +0200, Carl Eugen Hoyos wrote: > > Am Di., 1. Okt. 2019 um 21:35 Uhr schrieb Raphaël Zumer < > > rzu...@tebako.net>: > > > On Tue, 2019-10-01 at 20:09 +0100, Derek Buitenhuis wrote: > > > > Why not just write zero? > > > > > > > > It's, to me, worse to leave a bogus 64-bit write to appease > > > > bugs > > > > in > > > > our > > > > own demuxer. It's confusing and misleading for any readers of > > > > the > > > > code. > > > > > > In that case I would prefer changing the initial written value to > > > 0 > > > rather than 0xULL. Writing over the unused bytes > > > twice > > > to get around an old error is a bit odd as well. > > > > That may needlessly break non-seekable output. > > Writing a 0 as the initial value is consistent with the behaviour of > libvpx. > > libvpx writes 0 initially: > https://github.com/webmproject/libvpx/blob/v1.8.1/vpxenc.c#L1191 > then updates afterwards with the length (if output is seekable): > https://github.com/webmproject/libvpx/blob/v1.8.1/vpxenc.c#L1209 > > (for reference, the ivf_write_file_header function is here: > https://github.com/webmproject/libvpx/blob/v1.8.1/ivfenc.c#L16 ) > > So we need to make sure that ffmpeg can handle 0 values in this field > regardless. > For now I sent a patch in reply to Derek's message, which writes the length field as a 32-bit value explicitly, and zeroes out the unused bytes. But I agree that if ffmpeg cannot decode a zero length field properly, and this is how libvpx codes unseekable files, that is another problem. ___ 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".
[FFmpeg-devel] [PATCH] avformat/ivfenc: Change the length fields to 32 bits
There is no change in the encoded bitstream, but this ensures that the written field length is consistent with the reference implementation. Unused bytes are zeroed out for backwards compatibility. Signed-off-by: Raphaël Zumer --- libavformat/ivfenc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c index ae461a872b..eb70421c44 100644 --- a/libavformat/ivfenc.c +++ b/libavformat/ivfenc.c @@ -84,7 +84,8 @@ static int ivf_write_trailer(AVFormatContext *s) avio_seek(pb, 24, SEEK_SET); // overwrite the "length" field (duration) -avio_wl64(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1)); +avio_wl32(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1)); +avio_wl32(pb, 0); // zero out unused bytes avio_seek(pb, end, SEEK_SET); } -- 2.23.0 ___ 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] avformat/ivf: Change the length field to 32 bits
On Tue, 2019-10-01 at 20:09 +0100, Derek Buitenhuis wrote: > On 01/10/2019 18:25, James Almer wrote: > > The value in the unused field will be 0x after this change > > instead of 0, since you're writing 32 bits as duration instead of > > 64 > > where the high 32 bits (corresponding to the unused field) are > > zeroed. > > That means the ivf demuxer prior to this patch will read bogus > > duration > > values from ivf files created after this patch. > > > > Just leave the muxer as is. > > Why not just write zero? > > It's, to me, worse to leave a bogus 64-bit write to appease bugs in > our > own demuxer. It's confusing and misleading for any readers of the > code. In that case I would prefer changing the initial written value to 0 rather than 0xULL. Writing over the unused bytes twice to get around an old error is a bit odd as well. ___ 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] ivfdec/ivfenc: Match behaviour of libvpx and chromium
This is a superset of my patch(es). It should match the behavior of
libvpx more closely, but the validity of the change from duration to
number of frames depends on your interpretation of the reference
implementation, which comments the field as "length".
On Tue, 2019-10-01 at 23:41 +0530, Gyan wrote:
>
> On 01-10-2019 11:26 PM, Calvin Walton wrote:
> > The ffmpeg code read and wrote a 64bit duration field (in timebase
> > units) in the ivf
> > header, where the libvpx and chromium code instead use a 32bit
> > frame count field, and
> > then 32bits of unused (reserved?) space.
> >
> > Switch ffmpeg to match the behaviour of libvpx & chromium.
> >
> > Note that libvpx writes 0 to the frame count field when initially
> > writing the header
> > then seeks back and overwrites it with the real frame count. ffmpeg
> > used to write
> > 0x - I've changed the behaviour to match libvpx.
> >
> > References:
> > https://github.com/webmproject/libvpx/blob/v1.8.1/ivfenc.c#L16
> > Which is called from:
> > https://github.com/webmproject/libvpx/blob/v1.8.1/vpxenc.c#L1191
> > (initial header)
> > https://github.com/webmproject/libvpx/blob/v1.8.1/vpxenc.c#L1209
> > (rewrite with frame count)
> > And the chromium parser:
> > https://chromium.googlesource.com/chromium/src/media/+/1681b9abff73fe0e3d0932aefdab4f039a284d1a/filters/ivf_parser.h
> > ---
> > libavformat/ivfdec.c | 3 ++-
> > libavformat/ivfenc.c | 11 ---
> > 2 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/libavformat/ivfdec.c b/libavformat/ivfdec.c
> > index 40ae464b76..2eaa5164ff 100644
> > --- a/libavformat/ivfdec.c
> > +++ b/libavformat/ivfdec.c
> > @@ -53,7 +53,8 @@ static int read_header(AVFormatContext *s)
> > st->codecpar->height = avio_rl16(s->pb);
> > time_base.den = avio_rl32(s->pb);
> > time_base.num = avio_rl32(s->pb);
> > -st->duration = avio_rl64(s->pb);
> > +st->nb_frames = avio_rl32(s->pb);
> > +avio_skip(s->pb, 4); // 32 bits unused
> >
> > st->need_parsing = AVSTREAM_PARSE_HEADERS;
> >
> > diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c
> > index adf72117e9..85ca6045ba 100644
> > --- a/libavformat/ivfenc.c
> > +++ b/libavformat/ivfenc.c
> > @@ -22,8 +22,7 @@
> > #include "libavutil/intreadwrite.h"
> >
> > typedef struct IVFEncContext {
> > -unsigned frame_cnt;
> > -uint64_t last_pts, sum_delta_pts;
> > +uint32_t frame_cnt;
> > } IVFEncContext;
> >
> > static int ivf_write_header(AVFormatContext *s)
> > @@ -53,7 +52,8 @@ static int ivf_write_header(AVFormatContext *s)
> > avio_wl16(pb, par->height);
> > avio_wl32(pb, s->streams[0]->time_base.den);
> > avio_wl32(pb, s->streams[0]->time_base.num);
> > -avio_wl64(pb, 0xULL);
> > +avio_wl32(pb, 0); // frame count
> > +avio_wl32(pb, 0); // unused
> >
> > return 0;
> > }
> > @@ -66,10 +66,7 @@ static int ivf_write_packet(AVFormatContext *s,
> > AVPacket *pkt)
> > avio_wl32(pb, pkt->size);
> > avio_wl64(pb, pkt->pts);
> > avio_write(pb, pkt->data, pkt->size);
> > -if (ctx->frame_cnt)
> > -ctx->sum_delta_pts += pkt->pts - ctx->last_pts;
> > ctx->frame_cnt++;
> > -ctx->last_pts = pkt->pts;
> >
> > return 0;
> > }
> > @@ -83,7 +80,7 @@ static int ivf_write_trailer(AVFormatContext *s)
> > size_t end = avio_tell(pb);
> >
> > avio_seek(pb, 24, SEEK_SET);
> > -avio_wl64(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx-
> > >frame_cnt - 1));
> > +avio_wl32(pb, ctx->frame_cnt);
> > avio_seek(pb, end, SEEK_SET);
> > }
>
> See
> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-October/250871.html
>
> Gyan
> ___
> 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".
___
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] avformat/ivf: Change the length field to 32 bits
Thank you for the review. I have left the encoded value as 64 bits and split the patch into two in the v2 just sent: one for the decoder change in field size, and one for the encoder comments. On Tue, 2019-10-01 at 14:25 -0300, James Almer wrote: > On 10/1/2019 2:05 PM, Raphaël Zumer wrote: > > Signed-off-by: Raphaël Zumer > > --- > > libavformat/ivfdec.c | 3 ++- > > libavformat/ivfenc.c | 5 +++-- > > 2 files changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/libavformat/ivfdec.c b/libavformat/ivfdec.c > > index 40ae464b76..2fdb6f5a04 100644 > > --- a/libavformat/ivfdec.c > > +++ b/libavformat/ivfdec.c > > @@ -53,7 +53,8 @@ static int read_header(AVFormatContext *s) > > st->codecpar->height = avio_rl16(s->pb); > > time_base.den = avio_rl32(s->pb); > > time_base.num = avio_rl32(s->pb); > > -st->duration = avio_rl64(s->pb); > > +st->duration = avio_rl32(s->pb); > > +avio_rl32(s->pb); // unused > > avio_skip(s->pb, 4); > > This part is good either way. > > > > > st->need_parsing = AVSTREAM_PARSE_HEADERS; > > > > diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c > > index adf72117e9..e135a78213 100644 > > --- a/libavformat/ivfenc.c > > +++ b/libavformat/ivfenc.c > > @@ -53,7 +53,7 @@ static int ivf_write_header(AVFormatContext *s) > > avio_wl16(pb, par->height); > > avio_wl32(pb, s->streams[0]->time_base.den); > > avio_wl32(pb, s->streams[0]->time_base.num); > > -avio_wl64(pb, 0xULL); > > +avio_wl64(pb, 0xULL); // length is overwritten > > at the end of muxing > > > > return 0; > > } > > @@ -83,7 +83,8 @@ static int ivf_write_trailer(AVFormatContext *s) > > size_t end = avio_tell(pb); > > > > avio_seek(pb, 24, SEEK_SET); > > -avio_wl64(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx- > > >frame_cnt - 1)); > > +// overwrite the "length" field (duration) > > +avio_wl32(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx- > > >frame_cnt - 1)); > > The value in the unused field will be 0x after this change > instead of 0, since you're writing 32 bits as duration instead of 64 > where the high 32 bits (corresponding to the unused field) are > zeroed. > That means the ivf demuxer prior to this patch will read bogus > duration > values from ivf files created after this patch. > > Just leave the muxer as is. > > > avio_seek(pb, end, SEEK_SET); > > } > > > > > > ___ > 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". ___ 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".
[FFmpeg-devel] [PATCH v2 2/2] avformat/ivfenc: Comment the length field encoding process
Signed-off-by: Raphaël Zumer --- libavformat/ivfenc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c index adf72117e9..ae461a872b 100644 --- a/libavformat/ivfenc.c +++ b/libavformat/ivfenc.c @@ -53,7 +53,7 @@ static int ivf_write_header(AVFormatContext *s) avio_wl16(pb, par->height); avio_wl32(pb, s->streams[0]->time_base.den); avio_wl32(pb, s->streams[0]->time_base.num); -avio_wl64(pb, 0xULL); +avio_wl64(pb, 0xULL); // length is overwritten at the end of muxing return 0; } @@ -83,6 +83,7 @@ static int ivf_write_trailer(AVFormatContext *s) size_t end = avio_tell(pb); avio_seek(pb, 24, SEEK_SET); +// overwrite the "length" field (duration) avio_wl64(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1)); avio_seek(pb, end, SEEK_SET); } -- 2.23.0 ___ 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".
[FFmpeg-devel] [PATCH v2 1/2] avformat/ivfdec: Change the length field to 32 bits
Signed-off-by: Raphaël Zumer --- libavformat/ivfdec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavformat/ivfdec.c b/libavformat/ivfdec.c index 40ae464b76..4a802573e7 100644 --- a/libavformat/ivfdec.c +++ b/libavformat/ivfdec.c @@ -53,7 +53,8 @@ static int read_header(AVFormatContext *s) st->codecpar->height = avio_rl16(s->pb); time_base.den = avio_rl32(s->pb); time_base.num = avio_rl32(s->pb); -st->duration = avio_rl64(s->pb); +st->duration = avio_rl32(s->pb); +avio_skip(s->pb, 4); // unused st->need_parsing = AVSTREAM_PARSE_HEADERS; -- 2.23.0 ___ 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".
[FFmpeg-devel] [PATCH] avformat/ivf: Change the length field to 32 bits
Signed-off-by: Raphaël Zumer --- libavformat/ivfdec.c | 3 ++- libavformat/ivfenc.c | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libavformat/ivfdec.c b/libavformat/ivfdec.c index 40ae464b76..2fdb6f5a04 100644 --- a/libavformat/ivfdec.c +++ b/libavformat/ivfdec.c @@ -53,7 +53,8 @@ static int read_header(AVFormatContext *s) st->codecpar->height = avio_rl16(s->pb); time_base.den = avio_rl32(s->pb); time_base.num = avio_rl32(s->pb); -st->duration = avio_rl64(s->pb); +st->duration = avio_rl32(s->pb); +avio_rl32(s->pb); // unused st->need_parsing = AVSTREAM_PARSE_HEADERS; diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c index adf72117e9..e135a78213 100644 --- a/libavformat/ivfenc.c +++ b/libavformat/ivfenc.c @@ -53,7 +53,7 @@ static int ivf_write_header(AVFormatContext *s) avio_wl16(pb, par->height); avio_wl32(pb, s->streams[0]->time_base.den); avio_wl32(pb, s->streams[0]->time_base.num); -avio_wl64(pb, 0xULL); +avio_wl64(pb, 0xULL); // length is overwritten at the end of muxing return 0; } @@ -83,7 +83,8 @@ static int ivf_write_trailer(AVFormatContext *s) size_t end = avio_tell(pb); avio_seek(pb, 24, SEEK_SET); -avio_wl64(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1)); +// overwrite the "length" field (duration) +avio_wl32(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1)); avio_seek(pb, end, SEEK_SET); } -- 2.23.0 ___ 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".
[FFmpeg-devel] (no subject)
The IVF format includes a 4-byte field for the number of frames. I could not find a specification to cite, but for example, the Chromium IVF parser handles this field. Please see: https://chromium.googlesource.com/chromium/src/media/+/master/filters/ivf_parser.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/ivfenc: Encode the number of frames
Signed-off-by: Raphaël Zumer --- libavformat/ivfenc.c | 3 ++- libavformat/version.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c index adf72117e9..54327f5025 100644 --- a/libavformat/ivfenc.c +++ b/libavformat/ivfenc.c @@ -53,7 +53,8 @@ static int ivf_write_header(AVFormatContext *s) avio_wl16(pb, par->height); avio_wl32(pb, s->streams[0]->time_base.den); avio_wl32(pb, s->streams[0]->time_base.num); -avio_wl64(pb, 0xULL); +avio_wl32(pb, s->streams[0]->nb_frames); +avio_wl32(pb, 0xUL); return 0; } diff --git a/libavformat/version.h b/libavformat/version.h index bcd0408d28..426ffb16e4 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -33,7 +33,7 @@ // Also please add any ticket numbers that you believe might be affected here #define LIBAVFORMAT_VERSION_MAJOR 58 #define LIBAVFORMAT_VERSION_MINOR 33 -#define LIBAVFORMAT_VERSION_MICRO 100 +#define LIBAVFORMAT_VERSION_MICRO 101 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ LIBAVFORMAT_VERSION_MINOR, \ -- 2.23.0 ___ 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 1/3] avutil/pixfmt: Add EBU Tech. 3213-E AVColorPrimaries value
It's been about 3 weeks, so sending a reminder in case anyone can review
this.
On 2019-08-11 9:54 a.m., rzu...@tebako.net wrote:
From: Raphaël Zumer
This is an alias for JEDEC P22.
The name associated with the value is also changed
from "jedec-p22" to "ebu3213" to match ITU-T H.273.
Signed-off-by: Raphaël Zumer
---
doc/APIchanges | 3 +++
libavutil/pixdesc.c | 2 +-
libavutil/pixfmt.h | 3 ++-
libavutil/version.h | 2 +-
4 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 6603a8229e..f71b0c4a75 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
API changes, most recent first:
+2019-08-08 - xx - lavu 56.34.100 - pixfmt.h
+ Add EBU Tech. 3213-E AVColorPrimaries value
+
2019-07-27 - xx - lavu 56.33.100 - tx.h
Add AV_TX_DOUBLE_FFT and AV_TX_DOUBLE_MDCT
diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index b97b0665b0..05dd4a1e20 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -2369,7 +2369,7 @@ static const char * const
color_primaries_names[AVCOL_PRI_NB] = {
[AVCOL_PRI_SMPTE428] = "smpte428",
[AVCOL_PRI_SMPTE431] = "smpte431",
[AVCOL_PRI_SMPTE432] = "smpte432",
-[AVCOL_PRI_JEDEC_P22] = "jedec-p22",
+[AVCOL_PRI_EBU3213] = "ebu3213",
};
static const char * const color_transfer_names[] = {
diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index 8b54c9415b..d78e863d4b 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -456,7 +456,8 @@ enum AVColorPrimaries {
AVCOL_PRI_SMPTEST428_1 = AVCOL_PRI_SMPTE428,
AVCOL_PRI_SMPTE431= 11, ///< SMPTE ST 431-2 (2011) / DCI P3
AVCOL_PRI_SMPTE432= 12, ///< SMPTE ST 432-1 (2010) / P3 D65 / Display
P3
-AVCOL_PRI_JEDEC_P22 = 22, ///< JEDEC P22 phosphors
+AVCOL_PRI_EBU3213 = 22, ///< EBU Tech. 3213-E / JEDEC P22 phosphors
+AVCOL_PRI_JEDEC_P22 = AVCOL_PRI_EBU3213,
AVCOL_PRI_NB///< Not part of ABI
};
diff --git a/libavutil/version.h b/libavutil/version.h
index ecc6a7c9e2..658a508284 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
*/
#define LIBAVUTIL_VERSION_MAJOR 56
-#define LIBAVUTIL_VERSION_MINOR 33
+#define LIBAVUTIL_VERSION_MINOR 34
#define LIBAVUTIL_VERSION_MICRO 100
#define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
___
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 1/3] avutil/pixfmt: Add EBU Tech. 3213-E AVColorPrimaries value
I would like to mention that this set of patches is revised following
previous comments. Since I did not set the version number correctly, it
might have slipped past some people's radars. If there are any further
changes needed, please let me know.
RZ
On 2019-08-11 9:54 a.m., rzu...@tebako.net wrote:
From: Raphaël Zumer
This is an alias for JEDEC P22.
The name associated with the value is also changed
from "jedec-p22" to "ebu3213" to match ITU-T H.273.
Signed-off-by: Raphaël Zumer
---
doc/APIchanges | 3 +++
libavutil/pixdesc.c | 2 +-
libavutil/pixfmt.h | 3 ++-
libavutil/version.h | 2 +-
4 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 6603a8229e..f71b0c4a75 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
API changes, most recent first:
+2019-08-08 - xx - lavu 56.34.100 - pixfmt.h
+ Add EBU Tech. 3213-E AVColorPrimaries value
+
2019-07-27 - xx - lavu 56.33.100 - tx.h
Add AV_TX_DOUBLE_FFT and AV_TX_DOUBLE_MDCT
diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index b97b0665b0..05dd4a1e20 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -2369,7 +2369,7 @@ static const char * const
color_primaries_names[AVCOL_PRI_NB] = {
[AVCOL_PRI_SMPTE428] = "smpte428",
[AVCOL_PRI_SMPTE431] = "smpte431",
[AVCOL_PRI_SMPTE432] = "smpte432",
-[AVCOL_PRI_JEDEC_P22] = "jedec-p22",
+[AVCOL_PRI_EBU3213] = "ebu3213",
};
static const char * const color_transfer_names[] = {
diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index 8b54c9415b..d78e863d4b 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -456,7 +456,8 @@ enum AVColorPrimaries {
AVCOL_PRI_SMPTEST428_1 = AVCOL_PRI_SMPTE428,
AVCOL_PRI_SMPTE431= 11, ///< SMPTE ST 431-2 (2011) / DCI P3
AVCOL_PRI_SMPTE432= 12, ///< SMPTE ST 432-1 (2010) / P3 D65 / Display
P3
-AVCOL_PRI_JEDEC_P22 = 22, ///< JEDEC P22 phosphors
+AVCOL_PRI_EBU3213 = 22, ///< EBU Tech. 3213-E / JEDEC P22 phosphors
+AVCOL_PRI_JEDEC_P22 = AVCOL_PRI_EBU3213,
AVCOL_PRI_NB///< Not part of ABI
};
diff --git a/libavutil/version.h b/libavutil/version.h
index ecc6a7c9e2..658a508284 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
*/
#define LIBAVUTIL_VERSION_MAJOR 56
-#define LIBAVUTIL_VERSION_MINOR 33
+#define LIBAVUTIL_VERSION_MINOR 34
#define LIBAVUTIL_VERSION_MICRO 100
#define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
___
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 1/3] avutil/pixfmt: Add EBU Tech. 3213-E AVColorPrimaries value
On 2019-08-09 7:51 a.m., Vittorio Giovara wrote: this value was already present, this is just a rename I think it counts as a new enum value (variant), even though it aliases another. I initially wrote "rename", but reworded the commit message based on James Almer's comment: The subject should be something like "avutil/pixfmt: Add EBU Tech. 3213-E AVColorPrimaries value" I'd do the opposite, similarly to what is done for AVCOL_PRI_SMPTEST428_1 AVCOL_PRI_EBU3213 = 22, ///< JEDEC P22 phosphors, EBU Tech 3213 E AVCOL_PRI_JEDEC_P22 = AVCOL_PRI_EBU3213, AVCOL_PRI_NB///< Not part of ABI }; Will do. I will also replace internal usage of JEDEC_P22 with EBU3213 elsewhere. I just noticed I missed the same comment (and a few others) in JA's review earlier, sorry about that. Will apply all corrections in my next revision. ___ 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".
