| Message ID | GV1P250MB0737382DBD7B140F7AB307FE8FF92@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/7] avcodec/hevc/Makefile: Move rules for lavc/* files to lavc/Makefile | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_x86 | warning | New warnings during build |
Andreas Rheinhardt: > This is the VVC version of 8b5d15530127fea54e934043a64653859de07353. > > (Hint: This ensures that the order of NALU arrays is OPI-VPS-SPS-PPS- > Prefix-SEI-Suffix-SEI, regardless of the order in the original > extradata. I hope this is right.) > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavformat/vvc.c | 169 ++++++++++++++++++++-------------------------- > 1 file changed, 73 insertions(+), 96 deletions(-) > Even after these patches, there are some strange things going in vvc.c: 1. There are at least 71 bits in general_constraint_info (if present), yet only 70 of these are written (the gci_no_chroma_qp_offset_constraint_flag is not). 2. The code contains checks for VVC_DCI_NUT, yet these are dead, because these NALU type is filtered out before. 3. A comment claims to need at least one VPS and SPS, but it actually checks for SPS and PPS. Does someone know what was intended? - Andreas
On Wed, Jun 5, 2024 at 7:53 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > Andreas Rheinhardt: > > This is the VVC version of 8b5d15530127fea54e934043a64653859de07353. > > > > (Hint: This ensures that the order of NALU arrays is OPI-VPS-SPS-PPS- > > Prefix-SEI-Suffix-SEI, regardless of the order in the original > > extradata. I hope this is right.) > > > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > > --- > > libavformat/vvc.c | 169 ++++++++++++++++++++-------------------------- > > 1 file changed, 73 insertions(+), 96 deletions(-) > > > > Even after these patches, there are some strange things going in vvc.c: > 1. There are at least 71 bits in general_constraint_info (if present), > yet only 70 of these are written (the > gci_no_chroma_qp_offset_constraint_flag is not). > Not sure why this happened, but 14496-15 said this see the comment: /* * unsigned int (1) ptl_frame_only_constraint_flag * unsigned int (1) ptl_multilayer_enabled_flag * unsigned int (8*num_bytes_constraint_info -2) general_constraint_info */ 8*9-2 = 70. vvcc_parse_ptl parses the information from vps or sps. vvcc_write writes VvcDecoderConfigurationRecord, it's not a bit-to-bit map. I guess. 2. The code contains checks for VVC_DCI_NUT, yet these are dead, because > these NALU type is filtered out before. > it's from the spec, see "if (NAL_unit_type != DCI_NUT && NAL_unit_type != OPI_NUT)" in https://cloud.tencent.com/developer/article/2227294 num_nalus is inferred as 1 if it does not exist. Maybe we can add a check 3. A comment claims to need at least one VPS and SPS, but it actually > checks for SPS and PPS. Does someone know what was intended? > it's a typo. VPS is optional, SPS and PPS are mandatory. > > - Andreas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Wed, Jun 5, 2024 at 7:41 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > This is the VVC version of 8b5d15530127fea54e934043a64653859de07353. > > (Hint: This ensures that the order of NALU arrays is OPI-VPS-SPS-PPS- > Prefix-SEI-Suffix-SEI, regardless of the order in the original > extradata. I hope this is right.) > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavformat/vvc.c | 169 ++++++++++++++++++++-------------------------- > 1 file changed, 73 insertions(+), 96 deletions(-) > > diff --git a/libavformat/vvc.c b/libavformat/vvc.c > index 679bb07a4d..819ee02e2c 100644 > --- a/libavformat/vvc.c > +++ b/libavformat/vvc.c > @@ -32,6 +32,16 @@ > #include "avio_internal.h" > #include "vvc.h" > > +enum { > + OPI_INDEX, > + VPS_INDEX, > + SPS_INDEX, > + PPS_INDEX, > + SEI_PREFIX_INDEX, > + SEI_SUFFIX_INDEX, > + NB_ARRAYS > +}; > + > typedef struct VVCCNALUnitArray { > uint8_t array_completeness; > uint8_t NAL_unit_type; > @@ -67,7 +77,7 @@ typedef struct VVCDecoderConfigurationRecord { > uint16_t max_picture_height; > uint16_t avg_frame_rate; > uint8_t num_of_arrays; > - VVCCNALUnitArray *array; > + VVCCNALUnitArray arrays[NB_ARRAYS]; > } VVCDecoderConfigurationRecord; > > static void vvcc_update_ptl(VVCDecoderConfigurationRecord *vvcc, > @@ -432,32 +442,11 @@ static void nal_unit_parse_header(GetBitContext *gb, > uint8_t *nal_type) > > static int vvcc_array_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, > uint8_t nal_type, int > ps_array_completeness, > - VVCDecoderConfigurationRecord *vvcc) > + VVCCNALUnitArray *array) > { > int ret; > - uint8_t index; > uint16_t num_nalus; > - VVCCNALUnitArray *array; > - > - for (index = 0; index < vvcc->num_of_arrays; index++) > - if (vvcc->array[index].NAL_unit_type == nal_type) > - break; > - > - if (index >= vvcc->num_of_arrays) { > - uint8_t i; > - > - ret = > - av_reallocp_array(&vvcc->array, index + 1, > - sizeof(VVCCNALUnitArray)); > - if (ret < 0) > - return ret; > - > - for (i = vvcc->num_of_arrays; i <= index; i++) > - memset(&vvcc->array[i], 0, sizeof(VVCCNALUnitArray)); > - vvcc->num_of_arrays = index + 1; > - } > > - array = &vvcc->array[index]; > num_nalus = array->num_nalus; > > ret = av_reallocp_array(&array->nal_unit, num_nalus + 1, > sizeof(uint8_t *)); > @@ -504,7 +493,8 @@ static int vvcc_array_add_nal_unit(uint8_t *nal_buf, > uint32_t nal_size, > > static int vvcc_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, > int ps_array_completeness, > - VVCDecoderConfigurationRecord *vvcc) > + VVCDecoderConfigurationRecord *vvcc, > + unsigned array_idx) > { > int ret = 0; > GetBitContext gbc; > @@ -529,18 +519,15 @@ static int vvcc_add_nal_unit(uint8_t *nal_buf, > uint32_t nal_size, > * vvcc. Perhaps the SEI playload type should be checked > * and non-declarative SEI messages discarded? > */ > - switch (nal_type) { > - case VVC_OPI_NUT: > - case VVC_VPS_NUT: > - case VVC_SPS_NUT: > - case VVC_PPS_NUT: > - case VVC_PREFIX_SEI_NUT: > - case VVC_SUFFIX_SEI_NUT: > - ret = vvcc_array_add_nal_unit(nal_buf, nal_size, nal_type, > - ps_array_completeness, vvcc); > - if (ret < 0) > - goto end; > - else if (nal_type == VVC_VPS_NUT) > + ret = vvcc_array_add_nal_unit(nal_buf, nal_size, nal_type, > + ps_array_completeness, > + &vvcc->arrays[array_idx]); > + if (ret < 0) > + goto end; > + if (vvcc->arrays[array_idx].num_nalus == 1) > + vvcc->num_of_arrays++; > + > + if (nal_type == VVC_VPS_NUT) > ret = vvcc_parse_vps(&gbc, vvcc); > else if (nal_type == VVC_SPS_NUT) > ret = vvcc_parse_sps(&gbc, vvcc); > @@ -551,11 +538,6 @@ static int vvcc_add_nal_unit(uint8_t *nal_buf, > uint32_t nal_size, > } > if (ret < 0) > goto end; > - break; > - default: > - ret = AVERROR_INVALIDDATA; > - goto end; > - } > > end: > av_free(rbsp_buf); > @@ -572,22 +554,21 @@ static void vvcc_init(VVCDecoderConfigurationRecord > *vvcc) > > static void vvcc_close(VVCDecoderConfigurationRecord *vvcc) > { > - uint8_t i; > + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc->arrays); i++) { > + VVCCNALUnitArray *const array = &vvcc->arrays[i]; > > - for (i = 0; i < vvcc->num_of_arrays; i++) { > - vvcc->array[i].num_nalus = 0; > - av_freep(&vvcc->array[i].nal_unit); > - av_freep(&vvcc->array[i].nal_unit_length); > + array->num_nalus = 0; > + av_freep(&array->nal_unit); > + av_freep(&array->nal_unit_length); > } > > vvcc->num_of_arrays = 0; > - av_freep(&vvcc->array); > } > > static int vvcc_write(AVIOContext *pb, VVCDecoderConfigurationRecord > *vvcc) > { > uint8_t i; > - uint16_t j, vps_count = 0, sps_count = 0, pps_count = 0; > + uint16_t vps_count = 0, sps_count = 0, pps_count = 0; > /* > * It's unclear how to properly compute these fields, so > * let's always set them to values meaning 'unspecified'. > @@ -672,40 +653,33 @@ static int vvcc_write(AVIOContext *pb, > VVCDecoderConfigurationRecord *vvcc) > av_log(NULL, AV_LOG_TRACE, > "num_of_arrays: %" PRIu8 "\n", > vvcc->num_of_arrays); > - for (i = 0; i < vvcc->num_of_arrays; i++) { > + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc->arrays); i++) { > this will shadow the "uint8_t i" > + const VVCCNALUnitArray *const array = &vvcc->arrays[i]; > + > + if (array->num_nalus == 0) > + continue; > + > av_log(NULL, AV_LOG_TRACE, > - "array_completeness[%" PRIu8 "]: %" PRIu8 > "\n", i, > - vvcc->array[i].array_completeness); > + "array_completeness[%u]: %" PRIu8 "\n", i, > + array->array_completeness); > av_log(NULL, AV_LOG_TRACE, > - "NAL_unit_type[%" PRIu8 "]: %" PRIu8 > "\n", i, > - vvcc->array[i].NAL_unit_type); > + "NAL_unit_type[%u]: %" PRIu8 "\n", i, > + array->NAL_unit_type); > av_log(NULL, AV_LOG_TRACE, > - "num_nalus[%" PRIu8 "]: %" PRIu16 > "\n", i, > - vvcc->array[i].num_nalus); > - for (j = 0; j < vvcc->array[i].num_nalus; j++) > + "num_nalus[%u]: %" PRIu16 "\n", i, > + array->num_nalus); > + for (unsigned j = 0; j < array->num_nalus; j++) > av_log(NULL, AV_LOG_TRACE, > - "nal_unit_length[%" PRIu8 "][%" PRIu16 "]: > %" > - PRIu16 "\n", i, j, vvcc->array[i].nal_unit_length[j]); > + "nal_unit_length[%u][%u]: %" > + PRIu16 "\n", i, j, array->nal_unit_length[j]); > } > > /* > * We need at least one of each: VPS and SPS. > */ > - for (i = 0; i < vvcc->num_of_arrays; i++) > - switch (vvcc->array[i].NAL_unit_type) { > - case VVC_VPS_NUT: > - vps_count += vvcc->array[i].num_nalus; > - break; > - case VVC_SPS_NUT: > - sps_count += vvcc->array[i].num_nalus; > - break; > - case VVC_PPS_NUT: > - pps_count += vvcc->array[i].num_nalus; > - break; > - default: > - break; > - } > - > + vps_count = vvcc->arrays[VPS_INDEX].num_nalus; > + sps_count = vvcc->arrays[SPS_INDEX].num_nalus; > + pps_count = vvcc->arrays[PPS_INDEX].num_nalus; > if (vps_count > VVC_MAX_VPS_COUNT) > return AVERROR_INVALIDDATA; > if (!sps_count || sps_count > VVC_MAX_SPS_COUNT) > @@ -804,25 +778,29 @@ static int vvcc_write(AVIOContext *pb, > VVCDecoderConfigurationRecord *vvcc) > /* unsigned int(8) num_of_arrays; */ > avio_w8(pb, vvcc->num_of_arrays); > > - for (i = 0; i < vvcc->num_of_arrays; i++) { > + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc->arrays); i++) { > + const VVCCNALUnitArray *const array = &vvcc->arrays[i]; > + > + if (!array->num_nalus) > + continue; > /* > * bit(1) array_completeness; > * unsigned int(2) reserved = 0; > * unsigned int(5) NAL_unit_type; > */ > - avio_w8(pb, vvcc->array[i].array_completeness << 7 | > - vvcc->array[i].NAL_unit_type & 0x1f); > + avio_w8(pb, array->array_completeness << 7 | > + array->NAL_unit_type & 0x1f); > /* unsigned int(16) num_nalus; */ > - if (vvcc->array[i].NAL_unit_type != VVC_DCI_NUT && > - vvcc->array[i].NAL_unit_type != VVC_OPI_NUT) > - avio_wb16(pb, vvcc->array[i].num_nalus); > - for (j = 0; j < vvcc->array[i].num_nalus; j++) { > + if (array->NAL_unit_type != VVC_DCI_NUT && > + array->NAL_unit_type != VVC_OPI_NUT) > + avio_wb16(pb, array->num_nalus); > + for (int j = 0; j < array->num_nalus; j++) { > /* unsigned int(16) nal_unit_length; */ > - avio_wb16(pb, vvcc->array[i].nal_unit_length[j]); > + avio_wb16(pb, array->nal_unit_length[j]); > > /* bit(8*nal_unit_length) nal_unit; */ > - avio_write(pb, vvcc->array[i].nal_unit[j], > - vvcc->array[i].nal_unit_length[j]); > + avio_write(pb, array->nal_unit[j], > + array->nal_unit_length[j]); > } > } > > @@ -932,19 +910,18 @@ int ff_isom_write_vvcc(AVIOContext *pb, const > uint8_t *data, > > buf += 4; > > - switch (type) { > - case VVC_OPI_NUT: > - case VVC_VPS_NUT: > - case VVC_SPS_NUT: > - case VVC_PPS_NUT: > - case VVC_PREFIX_SEI_NUT: > - case VVC_SUFFIX_SEI_NUT: > - ret = vvcc_add_nal_unit(buf, len, ps_array_completeness, > &vvcc); > - if (ret < 0) > - goto end; > - break; > - default: > - break; > + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc.arrays); i++) { > + static const uint8_t array_idx_to_type[] = > + { VVC_OPI_NUT, VVC_VPS_NUT, VVC_SPS_NUT, > + VVC_PPS_NUT, VVC_PREFIX_SEI_NUT, VVC_SUFFIX_SEI_NUT }; > + > + if (type == array_idx_to_type[i]) { > + ret = vvcc_add_nal_unit(buf, len, ps_array_completeness, > + &vvcc, i); > + if (ret < 0) > + goto end; > + break; > + } > } > > buf += len; > -- > 2.40.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Nuo Mi: > On Wed, Jun 5, 2024 at 7:41 PM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> This is the VVC version of 8b5d15530127fea54e934043a64653859de07353. >> >> (Hint: This ensures that the order of NALU arrays is OPI-VPS-SPS-PPS- >> Prefix-SEI-Suffix-SEI, regardless of the order in the original >> extradata. I hope this is right.) >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavformat/vvc.c | 169 ++++++++++++++++++++-------------------------- >> 1 file changed, 73 insertions(+), 96 deletions(-) >> >> diff --git a/libavformat/vvc.c b/libavformat/vvc.c >> index 679bb07a4d..819ee02e2c 100644 >> --- a/libavformat/vvc.c >> +++ b/libavformat/vvc.c >> @@ -32,6 +32,16 @@ >> #include "avio_internal.h" >> #include "vvc.h" >> >> +enum { >> + OPI_INDEX, >> + VPS_INDEX, >> + SPS_INDEX, >> + PPS_INDEX, >> + SEI_PREFIX_INDEX, >> + SEI_SUFFIX_INDEX, >> + NB_ARRAYS >> +}; >> + >> typedef struct VVCCNALUnitArray { >> uint8_t array_completeness; >> uint8_t NAL_unit_type; >> @@ -67,7 +77,7 @@ typedef struct VVCDecoderConfigurationRecord { >> uint16_t max_picture_height; >> uint16_t avg_frame_rate; >> uint8_t num_of_arrays; >> - VVCCNALUnitArray *array; >> + VVCCNALUnitArray arrays[NB_ARRAYS]; >> } VVCDecoderConfigurationRecord; >> >> static void vvcc_update_ptl(VVCDecoderConfigurationRecord *vvcc, >> @@ -432,32 +442,11 @@ static void nal_unit_parse_header(GetBitContext *gb, >> uint8_t *nal_type) >> >> static int vvcc_array_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, >> uint8_t nal_type, int >> ps_array_completeness, >> - VVCDecoderConfigurationRecord *vvcc) >> + VVCCNALUnitArray *array) >> { >> int ret; >> - uint8_t index; >> uint16_t num_nalus; >> - VVCCNALUnitArray *array; >> - >> - for (index = 0; index < vvcc->num_of_arrays; index++) >> - if (vvcc->array[index].NAL_unit_type == nal_type) >> - break; >> - >> - if (index >= vvcc->num_of_arrays) { >> - uint8_t i; >> - >> - ret = >> - av_reallocp_array(&vvcc->array, index + 1, >> - sizeof(VVCCNALUnitArray)); >> - if (ret < 0) >> - return ret; >> - >> - for (i = vvcc->num_of_arrays; i <= index; i++) >> - memset(&vvcc->array[i], 0, sizeof(VVCCNALUnitArray)); >> - vvcc->num_of_arrays = index + 1; >> - } >> >> - array = &vvcc->array[index]; >> num_nalus = array->num_nalus; >> >> ret = av_reallocp_array(&array->nal_unit, num_nalus + 1, >> sizeof(uint8_t *)); >> @@ -504,7 +493,8 @@ static int vvcc_array_add_nal_unit(uint8_t *nal_buf, >> uint32_t nal_size, >> >> static int vvcc_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, >> int ps_array_completeness, >> - VVCDecoderConfigurationRecord *vvcc) >> + VVCDecoderConfigurationRecord *vvcc, >> + unsigned array_idx) >> { >> int ret = 0; >> GetBitContext gbc; >> @@ -529,18 +519,15 @@ static int vvcc_add_nal_unit(uint8_t *nal_buf, >> uint32_t nal_size, >> * vvcc. Perhaps the SEI playload type should be checked >> * and non-declarative SEI messages discarded? >> */ >> - switch (nal_type) { >> - case VVC_OPI_NUT: >> - case VVC_VPS_NUT: >> - case VVC_SPS_NUT: >> - case VVC_PPS_NUT: >> - case VVC_PREFIX_SEI_NUT: >> - case VVC_SUFFIX_SEI_NUT: >> - ret = vvcc_array_add_nal_unit(nal_buf, nal_size, nal_type, >> - ps_array_completeness, vvcc); >> - if (ret < 0) >> - goto end; >> - else if (nal_type == VVC_VPS_NUT) >> + ret = vvcc_array_add_nal_unit(nal_buf, nal_size, nal_type, >> + ps_array_completeness, >> + &vvcc->arrays[array_idx]); >> + if (ret < 0) >> + goto end; >> + if (vvcc->arrays[array_idx].num_nalus == 1) >> + vvcc->num_of_arrays++; >> + >> + if (nal_type == VVC_VPS_NUT) >> ret = vvcc_parse_vps(&gbc, vvcc); >> else if (nal_type == VVC_SPS_NUT) >> ret = vvcc_parse_sps(&gbc, vvcc); >> @@ -551,11 +538,6 @@ static int vvcc_add_nal_unit(uint8_t *nal_buf, >> uint32_t nal_size, >> } >> if (ret < 0) >> goto end; >> - break; >> - default: >> - ret = AVERROR_INVALIDDATA; >> - goto end; >> - } >> >> end: >> av_free(rbsp_buf); >> @@ -572,22 +554,21 @@ static void vvcc_init(VVCDecoderConfigurationRecord >> *vvcc) >> >> static void vvcc_close(VVCDecoderConfigurationRecord *vvcc) >> { >> - uint8_t i; >> + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc->arrays); i++) { >> + VVCCNALUnitArray *const array = &vvcc->arrays[i]; >> >> - for (i = 0; i < vvcc->num_of_arrays; i++) { >> - vvcc->array[i].num_nalus = 0; >> - av_freep(&vvcc->array[i].nal_unit); >> - av_freep(&vvcc->array[i].nal_unit_length); >> + array->num_nalus = 0; >> + av_freep(&array->nal_unit); >> + av_freep(&array->nal_unit_length); >> } >> >> vvcc->num_of_arrays = 0; >> - av_freep(&vvcc->array); >> } >> >> static int vvcc_write(AVIOContext *pb, VVCDecoderConfigurationRecord >> *vvcc) >> { >> uint8_t i; >> - uint16_t j, vps_count = 0, sps_count = 0, pps_count = 0; >> + uint16_t vps_count = 0, sps_count = 0, pps_count = 0; >> /* >> * It's unclear how to properly compute these fields, so >> * let's always set them to values meaning 'unspecified'. >> @@ -672,40 +653,33 @@ static int vvcc_write(AVIOContext *pb, >> VVCDecoderConfigurationRecord *vvcc) >> av_log(NULL, AV_LOG_TRACE, >> "num_of_arrays: %" PRIu8 "\n", >> vvcc->num_of_arrays); >> - for (i = 0; i < vvcc->num_of_arrays; i++) { >> + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc->arrays); i++) { >> > this will shadow the "uint8_t i" > I just sent a patch for this (and for fixing the comment). - Andreas
On Sun, Jun 9, 2024 at 5:14 AM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > Nuo Mi: > > On Wed, Jun 5, 2024 at 7:41 PM Andreas Rheinhardt < > > andreas.rheinhardt@outlook.com> wrote: > > > >> This is the VVC version of 8b5d15530127fea54e934043a64653859de07353. > >> > >> (Hint: This ensures that the order of NALU arrays is OPI-VPS-SPS-PPS- > >> Prefix-SEI-Suffix-SEI, regardless of the order in the original > >> extradata. I hope this is right.) > >> > >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > >> --- > >> libavformat/vvc.c | 169 ++++++++++++++++++++-------------------------- > >> 1 file changed, 73 insertions(+), 96 deletions(-) > >> > >> diff --git a/libavformat/vvc.c b/libavformat/vvc.c > >> index 679bb07a4d..819ee02e2c 100644 > >> --- a/libavformat/vvc.c > >> +++ b/libavformat/vvc.c > >> @@ -32,6 +32,16 @@ > >> #include "avio_internal.h" > >> #include "vvc.h" > >> > >> +enum { > >> + OPI_INDEX, > >> + VPS_INDEX, > >> + SPS_INDEX, > >> + PPS_INDEX, > >> + SEI_PREFIX_INDEX, > >> + SEI_SUFFIX_INDEX, > >> + NB_ARRAYS > >> +}; > >> + > >> typedef struct VVCCNALUnitArray { > >> uint8_t array_completeness; > >> uint8_t NAL_unit_type; > >> @@ -67,7 +77,7 @@ typedef struct VVCDecoderConfigurationRecord { > >> uint16_t max_picture_height; > >> uint16_t avg_frame_rate; > >> uint8_t num_of_arrays; > >> - VVCCNALUnitArray *array; > >> + VVCCNALUnitArray arrays[NB_ARRAYS]; > >> } VVCDecoderConfigurationRecord; > >> > >> static void vvcc_update_ptl(VVCDecoderConfigurationRecord *vvcc, > >> @@ -432,32 +442,11 @@ static void nal_unit_parse_header(GetBitContext > *gb, > >> uint8_t *nal_type) > >> > >> static int vvcc_array_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, > >> uint8_t nal_type, int > >> ps_array_completeness, > >> - VVCDecoderConfigurationRecord *vvcc) > >> + VVCCNALUnitArray *array) > >> { > >> int ret; > >> - uint8_t index; > >> uint16_t num_nalus; > >> - VVCCNALUnitArray *array; > >> - > >> - for (index = 0; index < vvcc->num_of_arrays; index++) > >> - if (vvcc->array[index].NAL_unit_type == nal_type) > >> - break; > >> - > >> - if (index >= vvcc->num_of_arrays) { > >> - uint8_t i; > >> - > >> - ret = > >> - av_reallocp_array(&vvcc->array, index + 1, > >> - sizeof(VVCCNALUnitArray)); > >> - if (ret < 0) > >> - return ret; > >> - > >> - for (i = vvcc->num_of_arrays; i <= index; i++) > >> - memset(&vvcc->array[i], 0, sizeof(VVCCNALUnitArray)); > >> - vvcc->num_of_arrays = index + 1; > >> - } > >> > >> - array = &vvcc->array[index]; > >> num_nalus = array->num_nalus; > >> > >> ret = av_reallocp_array(&array->nal_unit, num_nalus + 1, > >> sizeof(uint8_t *)); > >> @@ -504,7 +493,8 @@ static int vvcc_array_add_nal_unit(uint8_t *nal_buf, > >> uint32_t nal_size, > >> > >> static int vvcc_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, > >> int ps_array_completeness, > >> - VVCDecoderConfigurationRecord *vvcc) > >> + VVCDecoderConfigurationRecord *vvcc, > >> + unsigned array_idx) > >> { > >> int ret = 0; > >> GetBitContext gbc; > >> @@ -529,18 +519,15 @@ static int vvcc_add_nal_unit(uint8_t *nal_buf, > >> uint32_t nal_size, > >> * vvcc. Perhaps the SEI playload type should be checked > >> * and non-declarative SEI messages discarded? > >> */ > >> - switch (nal_type) { > >> - case VVC_OPI_NUT: > >> - case VVC_VPS_NUT: > >> - case VVC_SPS_NUT: > >> - case VVC_PPS_NUT: > >> - case VVC_PREFIX_SEI_NUT: > >> - case VVC_SUFFIX_SEI_NUT: > >> - ret = vvcc_array_add_nal_unit(nal_buf, nal_size, nal_type, > >> - ps_array_completeness, vvcc); > >> - if (ret < 0) > >> - goto end; > >> - else if (nal_type == VVC_VPS_NUT) > >> + ret = vvcc_array_add_nal_unit(nal_buf, nal_size, nal_type, > >> + ps_array_completeness, > >> + &vvcc->arrays[array_idx]); > >> + if (ret < 0) > >> + goto end; > >> + if (vvcc->arrays[array_idx].num_nalus == 1) > >> + vvcc->num_of_arrays++; > >> + > >> + if (nal_type == VVC_VPS_NUT) > >> ret = vvcc_parse_vps(&gbc, vvcc); > >> else if (nal_type == VVC_SPS_NUT) > >> ret = vvcc_parse_sps(&gbc, vvcc); > >> @@ -551,11 +538,6 @@ static int vvcc_add_nal_unit(uint8_t *nal_buf, > >> uint32_t nal_size, > >> } > >> if (ret < 0) > >> goto end; > >> - break; > >> - default: > >> - ret = AVERROR_INVALIDDATA; > >> - goto end; > >> - } > >> > >> end: > >> av_free(rbsp_buf); > >> @@ -572,22 +554,21 @@ static void > vvcc_init(VVCDecoderConfigurationRecord > >> *vvcc) > >> > >> static void vvcc_close(VVCDecoderConfigurationRecord *vvcc) > >> { > >> - uint8_t i; > >> + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc->arrays); i++) { > >> + VVCCNALUnitArray *const array = &vvcc->arrays[i]; > >> > >> - for (i = 0; i < vvcc->num_of_arrays; i++) { > >> - vvcc->array[i].num_nalus = 0; > >> - av_freep(&vvcc->array[i].nal_unit); > >> - av_freep(&vvcc->array[i].nal_unit_length); > >> + array->num_nalus = 0; > >> + av_freep(&array->nal_unit); > >> + av_freep(&array->nal_unit_length); > >> } > >> > >> vvcc->num_of_arrays = 0; > >> - av_freep(&vvcc->array); > >> } > >> > >> static int vvcc_write(AVIOContext *pb, VVCDecoderConfigurationRecord > >> *vvcc) > >> { > >> uint8_t i; > >> - uint16_t j, vps_count = 0, sps_count = 0, pps_count = 0; > >> + uint16_t vps_count = 0, sps_count = 0, pps_count = 0; > >> /* > >> * It's unclear how to properly compute these fields, so > >> * let's always set them to values meaning 'unspecified'. > >> @@ -672,40 +653,33 @@ static int vvcc_write(AVIOContext *pb, > >> VVCDecoderConfigurationRecord *vvcc) > >> av_log(NULL, AV_LOG_TRACE, > >> "num_of_arrays: %" PRIu8 "\n", > >> vvcc->num_of_arrays); > >> - for (i = 0; i < vvcc->num_of_arrays; i++) { > >> + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc->arrays); i++) { > >> > > this will shadow the "uint8_t i" > > > > I just sent a patch for this (and for fixing the comment). > LGTM Thank you, Andreas. > - Andreas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
diff --git a/libavformat/vvc.c b/libavformat/vvc.c index 679bb07a4d..819ee02e2c 100644 --- a/libavformat/vvc.c +++ b/libavformat/vvc.c @@ -32,6 +32,16 @@ #include "avio_internal.h" #include "vvc.h" +enum { + OPI_INDEX, + VPS_INDEX, + SPS_INDEX, + PPS_INDEX, + SEI_PREFIX_INDEX, + SEI_SUFFIX_INDEX, + NB_ARRAYS +}; + typedef struct VVCCNALUnitArray { uint8_t array_completeness; uint8_t NAL_unit_type; @@ -67,7 +77,7 @@ typedef struct VVCDecoderConfigurationRecord { uint16_t max_picture_height; uint16_t avg_frame_rate; uint8_t num_of_arrays; - VVCCNALUnitArray *array; + VVCCNALUnitArray arrays[NB_ARRAYS]; } VVCDecoderConfigurationRecord; static void vvcc_update_ptl(VVCDecoderConfigurationRecord *vvcc, @@ -432,32 +442,11 @@ static void nal_unit_parse_header(GetBitContext *gb, uint8_t *nal_type) static int vvcc_array_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, uint8_t nal_type, int ps_array_completeness, - VVCDecoderConfigurationRecord *vvcc) + VVCCNALUnitArray *array) { int ret; - uint8_t index; uint16_t num_nalus; - VVCCNALUnitArray *array; - - for (index = 0; index < vvcc->num_of_arrays; index++) - if (vvcc->array[index].NAL_unit_type == nal_type) - break; - - if (index >= vvcc->num_of_arrays) { - uint8_t i; - - ret = - av_reallocp_array(&vvcc->array, index + 1, - sizeof(VVCCNALUnitArray)); - if (ret < 0) - return ret; - - for (i = vvcc->num_of_arrays; i <= index; i++) - memset(&vvcc->array[i], 0, sizeof(VVCCNALUnitArray)); - vvcc->num_of_arrays = index + 1; - } - array = &vvcc->array[index]; num_nalus = array->num_nalus; ret = av_reallocp_array(&array->nal_unit, num_nalus + 1, sizeof(uint8_t *)); @@ -504,7 +493,8 @@ static int vvcc_array_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, static int vvcc_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, int ps_array_completeness, - VVCDecoderConfigurationRecord *vvcc) + VVCDecoderConfigurationRecord *vvcc, + unsigned array_idx) { int ret = 0; GetBitContext gbc; @@ -529,18 +519,15 @@ static int vvcc_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, * vvcc. Perhaps the SEI playload type should be checked * and non-declarative SEI messages discarded? */ - switch (nal_type) { - case VVC_OPI_NUT: - case VVC_VPS_NUT: - case VVC_SPS_NUT: - case VVC_PPS_NUT: - case VVC_PREFIX_SEI_NUT: - case VVC_SUFFIX_SEI_NUT: - ret = vvcc_array_add_nal_unit(nal_buf, nal_size, nal_type, - ps_array_completeness, vvcc); - if (ret < 0) - goto end; - else if (nal_type == VVC_VPS_NUT) + ret = vvcc_array_add_nal_unit(nal_buf, nal_size, nal_type, + ps_array_completeness, + &vvcc->arrays[array_idx]); + if (ret < 0) + goto end; + if (vvcc->arrays[array_idx].num_nalus == 1) + vvcc->num_of_arrays++; + + if (nal_type == VVC_VPS_NUT) ret = vvcc_parse_vps(&gbc, vvcc); else if (nal_type == VVC_SPS_NUT) ret = vvcc_parse_sps(&gbc, vvcc); @@ -551,11 +538,6 @@ static int vvcc_add_nal_unit(uint8_t *nal_buf, uint32_t nal_size, } if (ret < 0) goto end; - break; - default: - ret = AVERROR_INVALIDDATA; - goto end; - } end: av_free(rbsp_buf); @@ -572,22 +554,21 @@ static void vvcc_init(VVCDecoderConfigurationRecord *vvcc) static void vvcc_close(VVCDecoderConfigurationRecord *vvcc) { - uint8_t i; + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc->arrays); i++) { + VVCCNALUnitArray *const array = &vvcc->arrays[i]; - for (i = 0; i < vvcc->num_of_arrays; i++) { - vvcc->array[i].num_nalus = 0; - av_freep(&vvcc->array[i].nal_unit); - av_freep(&vvcc->array[i].nal_unit_length); + array->num_nalus = 0; + av_freep(&array->nal_unit); + av_freep(&array->nal_unit_length); } vvcc->num_of_arrays = 0; - av_freep(&vvcc->array); } static int vvcc_write(AVIOContext *pb, VVCDecoderConfigurationRecord *vvcc) { uint8_t i; - uint16_t j, vps_count = 0, sps_count = 0, pps_count = 0; + uint16_t vps_count = 0, sps_count = 0, pps_count = 0; /* * It's unclear how to properly compute these fields, so * let's always set them to values meaning 'unspecified'. @@ -672,40 +653,33 @@ static int vvcc_write(AVIOContext *pb, VVCDecoderConfigurationRecord *vvcc) av_log(NULL, AV_LOG_TRACE, "num_of_arrays: %" PRIu8 "\n", vvcc->num_of_arrays); - for (i = 0; i < vvcc->num_of_arrays; i++) { + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc->arrays); i++) { + const VVCCNALUnitArray *const array = &vvcc->arrays[i]; + + if (array->num_nalus == 0) + continue; + av_log(NULL, AV_LOG_TRACE, - "array_completeness[%" PRIu8 "]: %" PRIu8 "\n", i, - vvcc->array[i].array_completeness); + "array_completeness[%u]: %" PRIu8 "\n", i, + array->array_completeness); av_log(NULL, AV_LOG_TRACE, - "NAL_unit_type[%" PRIu8 "]: %" PRIu8 "\n", i, - vvcc->array[i].NAL_unit_type); + "NAL_unit_type[%u]: %" PRIu8 "\n", i, + array->NAL_unit_type); av_log(NULL, AV_LOG_TRACE, - "num_nalus[%" PRIu8 "]: %" PRIu16 "\n", i, - vvcc->array[i].num_nalus); - for (j = 0; j < vvcc->array[i].num_nalus; j++) + "num_nalus[%u]: %" PRIu16 "\n", i, + array->num_nalus); + for (unsigned j = 0; j < array->num_nalus; j++) av_log(NULL, AV_LOG_TRACE, - "nal_unit_length[%" PRIu8 "][%" PRIu16 "]: %" - PRIu16 "\n", i, j, vvcc->array[i].nal_unit_length[j]); + "nal_unit_length[%u][%u]: %" + PRIu16 "\n", i, j, array->nal_unit_length[j]); } /* * We need at least one of each: VPS and SPS. */ - for (i = 0; i < vvcc->num_of_arrays; i++) - switch (vvcc->array[i].NAL_unit_type) { - case VVC_VPS_NUT: - vps_count += vvcc->array[i].num_nalus; - break; - case VVC_SPS_NUT: - sps_count += vvcc->array[i].num_nalus; - break; - case VVC_PPS_NUT: - pps_count += vvcc->array[i].num_nalus; - break; - default: - break; - } - + vps_count = vvcc->arrays[VPS_INDEX].num_nalus; + sps_count = vvcc->arrays[SPS_INDEX].num_nalus; + pps_count = vvcc->arrays[PPS_INDEX].num_nalus; if (vps_count > VVC_MAX_VPS_COUNT) return AVERROR_INVALIDDATA; if (!sps_count || sps_count > VVC_MAX_SPS_COUNT) @@ -804,25 +778,29 @@ static int vvcc_write(AVIOContext *pb, VVCDecoderConfigurationRecord *vvcc) /* unsigned int(8) num_of_arrays; */ avio_w8(pb, vvcc->num_of_arrays); - for (i = 0; i < vvcc->num_of_arrays; i++) { + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc->arrays); i++) { + const VVCCNALUnitArray *const array = &vvcc->arrays[i]; + + if (!array->num_nalus) + continue; /* * bit(1) array_completeness; * unsigned int(2) reserved = 0; * unsigned int(5) NAL_unit_type; */ - avio_w8(pb, vvcc->array[i].array_completeness << 7 | - vvcc->array[i].NAL_unit_type & 0x1f); + avio_w8(pb, array->array_completeness << 7 | + array->NAL_unit_type & 0x1f); /* unsigned int(16) num_nalus; */ - if (vvcc->array[i].NAL_unit_type != VVC_DCI_NUT && - vvcc->array[i].NAL_unit_type != VVC_OPI_NUT) - avio_wb16(pb, vvcc->array[i].num_nalus); - for (j = 0; j < vvcc->array[i].num_nalus; j++) { + if (array->NAL_unit_type != VVC_DCI_NUT && + array->NAL_unit_type != VVC_OPI_NUT) + avio_wb16(pb, array->num_nalus); + for (int j = 0; j < array->num_nalus; j++) { /* unsigned int(16) nal_unit_length; */ - avio_wb16(pb, vvcc->array[i].nal_unit_length[j]); + avio_wb16(pb, array->nal_unit_length[j]); /* bit(8*nal_unit_length) nal_unit; */ - avio_write(pb, vvcc->array[i].nal_unit[j], - vvcc->array[i].nal_unit_length[j]); + avio_write(pb, array->nal_unit[j], + array->nal_unit_length[j]); } } @@ -932,19 +910,18 @@ int ff_isom_write_vvcc(AVIOContext *pb, const uint8_t *data, buf += 4; - switch (type) { - case VVC_OPI_NUT: - case VVC_VPS_NUT: - case VVC_SPS_NUT: - case VVC_PPS_NUT: - case VVC_PREFIX_SEI_NUT: - case VVC_SUFFIX_SEI_NUT: - ret = vvcc_add_nal_unit(buf, len, ps_array_completeness, &vvcc); - if (ret < 0) - goto end; - break; - default: - break; + for (unsigned i = 0; i < FF_ARRAY_ELEMS(vvcc.arrays); i++) { + static const uint8_t array_idx_to_type[] = + { VVC_OPI_NUT, VVC_VPS_NUT, VVC_SPS_NUT, + VVC_PPS_NUT, VVC_PREFIX_SEI_NUT, VVC_SUFFIX_SEI_NUT }; + + if (type == array_idx_to_type[i]) { + ret = vvcc_add_nal_unit(buf, len, ps_array_completeness, + &vvcc, i); + if (ret < 0) + goto end; + break; + } } buf += len;
This is the VVC version of 8b5d15530127fea54e934043a64653859de07353. (Hint: This ensures that the order of NALU arrays is OPI-VPS-SPS-PPS- Prefix-SEI-Suffix-SEI, regardless of the order in the original extradata. I hope this is right.) Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/vvc.c | 169 ++++++++++++++++++++-------------------------- 1 file changed, 73 insertions(+), 96 deletions(-)