>
>> > + return AVERROR(ENOMEM);
>> > + }
>> > + h266->ph = (H266RawPH*)h266->ph_ref->data;
>> > + memcpy(h266->ph, ¤t->sh_picture_header,
>> sizeof(H266RawPH));
>>
>> This memcpy() is naughty - if that was a ref to a previous picture header
>> then you've just overwritten the reference while the other unit still holds
>> it.
>>
>> If the slice is unit->content then unit->content_ref is a reference to a
>> superstructure of the picture header, avoiding the copy, but I'm not sure
>> it's the right way to do it.
>>
>> Is it right that the two possible options here are:
>> * Store a ref, which might be to a newly-allocated buffer or might be to
>> a previous unit.
>>
> * Store the whole structure in the private context.
>>
> ?
>>
>> If the structure isn't too large, then the second option feels much
>> simpler.
>>
>> If it is large such that holding references and avoiding copies is
>> beneficial (which is quite large, every ref entails a malloc/free
>> call-pair), then we probably want that ref to always be to the unit?
>>
> If we store the whole structure, it's hard to detect the picture header
> missing case.
>
Also, if we use this as start point of the native decoder, the native
decoder may need ref the picture header for multi-thread decoding.
>
>> > + }
>> > +
>> > + ph = h266->ph;
>> > + if (!ph) {
>> > + av_log(ctx->log_ctx, AV_LOG_ERROR, "Picture header not
>> available.\n");
>> > + return AVERROR_INVALIDDATA;
>> > + }
>> > ...
>> > diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h
>> > index a392880036..118b1052d4 100644
>> > --- a/libavcodec/cbs_internal.h
>> > +++ b/libavcodec/cbs_internal.h
>> > @@ -40,7 +40,7 @@ enum CBSContentType {
>> > enum {
>> > // Maximum number of unit types described by the same unit type
>> > // descriptor.
>> > - CBS_MAX_UNIT_TYPES = 3,
>> > + CBS_MAX_UNIT_TYPES = 4,
>>
>> Heh, fair :)
>>
>> > // Maximum number of reference buffer offsets in any one unit.
>> > CBS_MAX_REF_OFFSETS = 2,
>> > // Special value used in a unit type descriptor to indicate
>> that it
>> > @@ -204,6 +204,7 @@ int ff_cbs_write_signed(CodedBitstreamContext *ctx,
>> PutBitContext *pbc,
>> > extern const CodedBitstreamType ff_cbs_type_av1;
>> > extern const CodedBitstreamType ff_cbs_type_h264;
>> > extern const CodedBitstreamType ff_cbs_type_h265;
>> > +extern const CodedBitstreamType ff_cbs_type_h266;
>> > extern const CodedBitstreamType ff_cbs_type_jpeg;
>> > extern const CodedBitstreamType ff_cbs_type_mpeg2;
>> > extern const CodedBitstreamType ff_cbs_type_vp9;
>> > diff --git a/libavcodec/cbs_sei.c b/libavcodec/cbs_sei.c
>> > index c49830ad77..96c60259ce 100644
>> > --- a/libavcodec/cbs_sei.c
>> > +++ b/libavcodec/cbs_sei.c
>> > @@ -20,6 +20,7 @@
>> > #include "cbs_internal.h"
>> > #include "cbs_h264.h"
>> > #include "cbs_h265.h"
>> > +#include "cbs_h266.h"
>> > #include "cbs_sei.h"
>> >
>> > static void cbs_free_user_data_registered(void *opaque, uint8_t *data)
>> > @@ -132,6 +133,13 @@ static int cbs_sei_get_unit(CodedBitstreamContext
>> *ctx,
>> > else
>> > sei_type = HEVC_NAL_SEI_SUFFIX;
>> > break;
>> > + case AV_CODEC_ID_H266:
>> > + highest_vcl_type = VVC_RSV_IRAP_11;
>> > + if (prefix)
>> > + sei_type = VVC_PREFIX_SEI_NUT;
>> > + else
>> > + sei_type = VVC_SUFFIX_SEI_NUT;
>> > + break;
>> > default:
>> > return AVERROR(EINVAL);
>> > }
>> > @@ -207,6 +215,18 @@ static int cbs_sei_get_unit(CodedBitstreamContext
>> *ctx,
>> > memcpy(unit->content, &sei, sizeof(sei));
>> > }
>> > break;
>> > + case AV_CODEC_ID_H266:
>> > + {
>> > + H266RawSEI sei = {
>> > + .nal_unit_header = {
>> > + .nal_unit_type = sei_type,
>> > + .nuh_layer_id = 0,
>> > + .nuh_temporal_id_plus1 = 1,
>> > + },
>> > + };
>> > + memcpy(unit->content, &sei, sizeof(sei));
>> > + }
>> > + break;
>> > default:
>> > av_assert0(0);
>> > }
>> > @@ -237,6 +257,15 @@ static int
>> cbs_sei_get_message_list(CodedBitstreamContext *ctx,
>> > *list = &sei->message_list;
>> > }
>> > break;
>> > + case AV_CODEC_ID_H266:
>> > + {
>> > + H266RawSEI *sei = unit->content;
>> > + if (unit->type != VVC_PREFIX_SEI_NUT &&
>> > + unit->type != VVC_SUFFIX_SEI_NUT)
>> > + return AVERROR(EINVAL);
>> > + *list = &sei->message_list;
>> > + }
>> > + break;
>>
>> Yay, I'm glad the SEI stuff fitted in nicely with no extra weird cases.
>>
> Yes. It's a good solution to remove duplicate things
>
>> > default:
>> > return AVERROR(EINVAL);
>> > }
>> >
>> Thanks,
>>
>> - Mark
>> _______________________________________________
>> 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".
