[FFmpeg-devel] [PATCH 1/2] cbs_h265: read/write HEVC PREFIX SEI
Mark Thompson
sw at jkqxz.net
Sat Apr 21 22:40:53 EEST 2018
On 20/04/18 08:27, Haihao Xiang wrote:
> Similar to cbs_h264, cbs_h265_{read, write}_nal_unit() can handle HEVC
> prefix SEI NAL units now. Currently mastering display colour volume SEI
> message is added only, we may add more SEI message if needed later
>
> Signed-off-by: Haihao Xiang <haihao.xiang at intel.com>
> ---
> libavcodec/cbs_h2645.c | 43 ++++++++++
> libavcodec/cbs_h265.h | 38 +++++++++
> libavcodec/cbs_h265_syntax_template.c | 150 ++++++++++++++++++++++++++++++++++
> 3 files changed, 231 insertions(+)
>
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 5585831cf6..6fc5832966 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -29,6 +29,7 @@
> #include "h264_sei.h"
> #include "h2645_parse.h"
> #include "hevc.h"
> +#include "hevc_sei.h"
>
>
> static int cbs_read_ue_golomb(CodedBitstreamContext *ctx, GetBitContext *gbc,
> @@ -465,6 +466,26 @@ static void cbs_h265_free_slice(void *unit, uint8_t *content)
> av_freep(&content);
> }
>
> +static void cbs_h265_free_sei_payload(H265RawSEIPayload *payload)
> +{
> + switch (payload->payload_type) {
> + case HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO:
> + break;
> + default:
> + av_buffer_unref(&payload->payload.other.data_ref);
> + break;
> + }
> +}
> +
> +static void cbs_h265_free_sei(void *unit, uint8_t *content)
> +{
> + H265RawSEI *sei = (H265RawSEI*)content;
> + int i;
> + for (i = 0; i < sei->payload_count; i++)
> + cbs_h265_free_sei_payload(&sei->payload[i]);
> + av_freep(&content);
> +}
> +
> static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
> CodedBitstreamFragment *frag,
> const H2645Packet *packet)
> @@ -972,6 +993,20 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx,
> }
> break;
>
> + case HEVC_NAL_SEI_PREFIX:
I don't think it would be too hard to handling SEI_SUFFIX here too? (The different set of valid payloads doesn't matter when we aren't checking that below.)
> + err = ff_cbs_alloc_unit_content(ctx, unit, sizeof(H265RawSEI),
> + &cbs_h265_free_sei);
> +
> + if (err < 0)
> + return err;
> +
> + err = cbs_h265_read_sei(ctx, &gbc, unit->content);
> +
> + if (err < 0)
> + return err;
> +
> + break;
> +
> default:
> return AVERROR(ENOSYS);
> }
> @@ -1212,6 +1247,14 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx,
> }
> break;
>
> + case HEVC_NAL_SEI_PREFIX:
> + err = cbs_h265_write_sei(ctx, pbc, unit->content);
> +
> + if (err < 0)
> + return err;
> +
> + break;
> +
Please make these cases match the styling of the others.
> default:
> av_log(ctx->log_ctx, AV_LOG_ERROR, "Write unimplemented for "
> "NAL unit type %"PRIu32".\n", unit->type);
> diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h
> index 33e71fc234..e37c1b8d94 100644
> --- a/libavcodec/cbs_h265.h
> +++ b/libavcodec/cbs_h265.h
> @@ -25,6 +25,14 @@
> #include "cbs_h2645.h"
> #include "hevc.h"
>
> +enum {
> + // This limit is arbitrary - it is sufficient for one message of each
> + // type plus some repeats, and will therefore easily cover all sane
> + // streams. However, it is possible to make technically-valid streams
> + // for which it will fail (for example, by including a large number of
> + // user-data-unregistered messages).
> + H265_MAX_SEI_PAYLOADS = 64,
> +};
>
> typedef struct H265RawNALUnitHeader {
> uint8_t forbidden_zero_bit;
> @@ -516,6 +524,36 @@ typedef struct H265RawSlice {
> AVBufferRef *data_ref;
> } H265RawSlice;
>
> +typedef struct H265RawSEIMasteringDiplayColorVolume {
The H.265 standard uses English spelling of colour, not USAian.
> + struct {
> + uint16_t x;
> + uint16_t y;
> + } display_primaries[3];
Make it two arrays, display_primaries_x and display_primaries_y, so that it matches the standard.
> + uint16_t white_point_x;
> + uint16_t white_point_y;
> + uint32_t max_display_mastering_luminance;
> + uint32_t min_display_mastering_luminance;
> +} H265RawSEIMasteringDiplayColorVolume;
> +
> +typedef struct H265RawSEIPayload {
> + uint32_t payload_type;
> + uint32_t payload_size;
> + union {
> + H265RawSEIMasteringDiplayColorVolume mastering_display;
> + struct {
> + uint8_t *data;
> + size_t data_length;
> + AVBufferRef *data_ref;
> + } other;
> + } payload;
> +} H265RawSEIPayload;
> +
> +typedef struct H265RawSEI {
> + H265RawNALUnitHeader nal_unit_header;
> +
> + H265RawSEIPayload payload[H265_MAX_SEI_PAYLOADS];
> + uint8_t payload_count;
> +} H265RawSEI;
>
> typedef struct CodedBitstreamH265Context {
> // Reader/writer context in common with the H.264 implementation.
> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
> index 140c827c9d..cbe8f30be0 100644
> --- a/libavcodec/cbs_h265_syntax_template.c
> +++ b/libavcodec/cbs_h265_syntax_template.c
> @@ -1501,3 +1501,153 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw,
>
> return 0;
> }
> +
> +static int FUNC(sei_mastering_display)(CodedBitstreamContext *ctx, RWContext *rw,
> + H265RawSEIMasteringDiplayColorVolume *current)
> +{
> + int err, i;
> +
> + for (i = 0; i < 3; i++) {
This variable is called 'c' in the standard (which matters when it ends up in the trace name).
> + xu(16, display_primaries_x, current->display_primaries[i].x, 0, 50000);
> + xu(16, display_primaries_y, current->display_primaries[i].y, 0, 50000);
> + }
> +
> + xu(16, white_point_x, current->white_point_x, 0, 50000);
> + xu(16, white_point_y, current->white_point_y, 0, 50000);
> +
> + xu(32, max_display_mastering_luminance, current->max_display_mastering_luminance, 0, 0xFFFFFFFF);
MAX_UINT_BITS(32)
Also break the overlong line.
> + xu(32, min_display_mastering_luminance, current->min_display_mastering_luminance, 0, 0xFFFFFFFF);
You could use "min_display_mastering_luminance shall be less than max_display_mastering_luminance" as a bound here?
All of these should use the u() macro rather than xu(), since they aren't writing to a different variable.
> +
> + return 0;
> +}
> +
> +static int FUNC(sei_payload)(CodedBitstreamContext *ctx, RWContext *rw,
> + H265RawSEIPayload *current)
> +{
> + int err, i;
> + int start_position, end_position;
> +
> +#ifdef READ
> + start_position = get_bits_count(rw);
> +#else
> + start_position = put_bits_count(rw);
> +#endif
> +
> + switch (current->payload_type) {
> + case HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO:
> + CHECK(FUNC(sei_mastering_display)
> + (ctx, rw, ¤t->payload.mastering_display));
> +
> + break;
> +
> + default:
> + allocate(current->payload.other.data, current->payload_size);
> +
> + for (i = 0; i < current->payload_size; i++)
> + xu(8, payload_byte, current->payload.other.data[i], 0, 255);
> + }
> +
> + if (byte_alignment(rw)) {
> + av_unused int one = 1, zero = 0;
> + xu(1, bit_equal_to_one, one, 1, 1);
> + while (byte_alignment(rw))
> + xu(1, bit_equal_to_zero, zero, 0, 0);
> + }
> +
> +#ifdef READ
> + end_position = get_bits_count(rw);
> + if (end_position < start_position + 8 * current->payload_size) {
> + av_log(ctx->log_ctx, AV_LOG_ERROR, "Incorrect SEI payload length: "
> + "header %"PRIu32" bits, actually %d bits.\n",
> + 8 * current->payload_size,
> + end_position - start_position);
> + return AVERROR_INVALIDDATA;
> + }
> +#else
> + end_position = put_bits_count(rw);
> + current->payload_size = (end_position - start_position) >> 3;
> +#endif
> +
> + return 0;
> +}
> +
> +static int FUNC(sei)(CodedBitstreamContext *ctx, RWContext *rw,
> + H265RawSEI *current)
> +{
> + int err, k;
> +
> + HEADER("Supplemental Enhancement Information");
> +
> + CHECK(FUNC(nal_unit_header)(ctx, rw, ¤t->nal_unit_header, HEVC_NAL_SEI_PREFIX));
> +
> +#ifdef READ
> + for (k = 0; k < H265_MAX_SEI_PAYLOADS; k++) {
> + uint32_t payload_type = 0;
> + uint32_t payload_size = 0;
> + uint32_t tmp;
> +
> + while (show_bits(rw, 8) == 0xff) {
> + xu(8, ff_byte, tmp, 0xff, 0xff);
> + payload_type += 255;
> + }
> + xu(8, last_payload_type_byte, tmp, 0, 254);
> + payload_type += tmp;
> +
> + while (show_bits(rw, 8) == 0xff) {
> + xu(8, ff_byte, tmp, 0xff, 0xff);
> + payload_size += 255;
> + }
> + xu(8, last_payload_size_byte, tmp, 0, 254);
> + payload_size += tmp;
> +
> + current->payload[k].payload_type = payload_type;
> + current->payload[k].payload_size = payload_size;
> +
> + CHECK(FUNC(sei_payload)(ctx, rw, ¤t->payload[k]));
> +
> + if (!cbs_h2645_read_more_rbsp_data(rw))
> + break;
> + }
> + if (k >= H265_MAX_SEI_PAYLOADS) {
> + av_log(ctx->log_ctx, AV_LOG_ERROR, "Too many payloads in "
> + "SEI message: found %d.\n", k);
> + return AVERROR_INVALIDDATA;
> + }
> + current->payload_count = k + 1;
> +#else
> + for (k = 0; k < current->payload_count; k++) {
> + PutBitContext start_state;
> + uint32_t tmp;
> + int need_size, i;
> +
> + // Somewhat clumsy: we write the payload twice when
> + // we don't know the size in advance. This will mess
> + // with trace output, but is otherwise harmless.
> + start_state = *rw;
> + need_size = !current->payload[k].payload_size;
> + for (i = 0; i < 1 + need_size; i++) {
> + *rw = start_state;
> +
> + tmp = current->payload[k].payload_type;
> + while (tmp >= 255) {
> + xu(8, ff_byte, 0xff, 0xff, 0xff);
> + tmp -= 255;
> + }
> + xu(8, last_payload_type_byte, tmp, 0, 254);
> +
> + tmp = current->payload[k].payload_size;
> + while (tmp >= 255) {
> + xu(8, ff_byte, 0xff, 0xff, 0xff);
> + tmp -= 255;
> + }
> + xu(8, last_payload_size_byte, tmp, 0, 254);
> +
> + CHECK(FUNC(sei_payload)(ctx, rw, ¤t->payload[k]));
> + }
> + }
> +#endif
I admit you're copying what I wrote in the equivalent code for H.264, but this is still kindof horrible. Any thoughts on how to do this more nicely are welcome...
> +
> + CHECK(FUNC(rbsp_trailing_bits)(ctx, rw));
> +
> + return 0;
> +}
>
Looks generally ok, thank you for working on this!
- Mark
More information about the ffmpeg-devel
mailing list