| Message ID | 20240403190053.86712-1-post@frankplowman.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] lavc/vvc: Error if SPS ID is duplicated within CVS | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Hi Frank, Thank you for the patch. On Thu, Apr 4, 2024 at 3:01 AM Frank Plowman <post@frankplowman.com> wrote: > Key line from the spec is: > > "All SPS NAL units with a particular value of sps_seq_parameter_set_id > in a CVS shall have the same content." > > Prior to this patch, the VVC decoder's behaviour on encountering a > duplicated SPS ID (within the entire bitstream, not restricted to > a CVS) was simply to replace the entry in the SPS lookup table with the > new data. Illegal bitstreams with multiple SPSs in the same CVS sharing > an ID but differing elsewhere could cause all manner of issues. > > The patch tracks which SPS IDs have been used in the given CVS using the > new sps_id_used field of VVCParamSets. If it encounters an SPS with an > ID already in use and whose content differs from the previous SPS, it > throws an AVERROR_INVALIDDATA. > --- > libavcodec/vvc/vvc_ps.c | 29 +++++++++++++++++++++-------- > libavcodec/vvc/vvc_ps.h | 1 + > 2 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c > index 301fa16400..7a8daaa6ad 100644 > --- a/libavcodec/vvc/vvc_ps.c > +++ b/libavcodec/vvc/vvc_ps.c > @@ -219,14 +219,23 @@ fail: > return NULL; > } > > -static int decode_sps(VVCParamSets *ps, const H266RawSPS *rsps, void > *log_ctx) > +static int decode_sps(VVCParamSets *ps, const H266RawSPS *rsps, void > *log_ctx, int is_clvss) > { > const int sps_id = rsps->sps_seq_parameter_set_id; > const VVCSPS *old_sps = ps->sps_list[sps_id]; > const VVCSPS *sps; > > - if (old_sps && old_sps->r == rsps) > - return 0; > + if (is_clvss) { > + for (int sps_id = 0; sps_id < VVC_MAX_SPS_COUNT; ++sps_id) > + ps->sps_id_used[sps_id] = 0; > + } > + > + if (old_sps) > better use a bracket here. > + if (old_sps->r == rsps || !memcmp(old_sps->r, rsps, > sizeof(*old_sps->r))) > + return 0; > + else if (ps->sps_id_used[sps_id]) > + return AVERROR_INVALIDDATA; > + trailing space. > > > sps = sps_alloc(rsps, log_ctx); > if (!sps) > @@ -234,6 +243,7 @@ static int decode_sps(VVCParamSets *ps, const > H266RawSPS *rsps, void *log_ctx) > > ff_refstruct_unref(&ps->sps_list[sps_id]); > ps->sps_list[sps_id] = sps; > + ps->sps_id_used[sps_id] = 1; > > return 0; > } > @@ -610,7 +620,7 @@ static int decode_pps(VVCParamSets *ps, const > H266RawPPS *rpps) > return ret; > } > > -static int decode_ps(VVCParamSets *ps, const CodedBitstreamH266Context > *h266, void *log_ctx) > +static int decode_ps(VVCParamSets *ps, const CodedBitstreamH266Context > *h266, void *log_ctx, int is_clvss) > { > const H266RawPictureHeader *ph = h266->ph; > const H266RawPPS *rpps; > @@ -628,7 +638,7 @@ static int decode_ps(VVCParamSets *ps, const > CodedBitstreamH266Context *h266, vo > if (!rsps) > return AVERROR_INVALIDDATA; > > - ret = decode_sps(ps, rsps, log_ctx); > + ret = decode_sps(ps, rsps, log_ctx, is_clvss); > if (ret < 0) > return ret; > > @@ -867,13 +877,16 @@ int ff_vvc_decode_frame_ps(VVCFrameParamSets *fps, > struct VVCContext *s) > int ret = 0; > VVCParamSets *ps = &s->ps; > const CodedBitstreamH266Context *h266 = s->cbc->priv_data; > + int is_clvss; > > - ret = decode_ps(ps, h266, s->avctx); > + decode_recovery_flag(s); > + is_clvss = IS_CLVSS(s); > + > + ret = decode_ps(ps, h266, s->avctx, is_clvss); > if (ret < 0) > return ret; > > - decode_recovery_flag(s); > - ret = decode_frame_ps(fps, ps, h266, s->poc_tid0, IS_CLVSS(s)); > + ret = decode_frame_ps(fps, ps, h266, s->poc_tid0, is_clvss); > decode_recovery_poc(s, &fps->ph); > return ret; > } > diff --git a/libavcodec/vvc/vvc_ps.h b/libavcodec/vvc/vvc_ps.h > index f60d8b81c6..b293043de8 100644 > --- a/libavcodec/vvc/vvc_ps.h > +++ b/libavcodec/vvc/vvc_ps.h > @@ -210,6 +210,7 @@ typedef struct VVCLMCS { > > typedef struct VVCParamSets { > const VVCSPS *sps_list[VVC_MAX_SPS_COUNT]; ///< > RefStruct reference > + int sps_id_used[VVC_MAX_SPS_COUNT]; > The int array seems excessive for just a flag. Let's replace it with a uint16_t. Also, please relocate sps_id_used to the end of the structure." > const VVCPPS *pps_list[VVC_MAX_PPS_COUNT]; ///< > RefStruct reference > const VVCALF *alf_list[VVC_MAX_ALF_COUNT]; ///< > RefStruct reference > const H266RawAPS *lmcs_list[VVC_MAX_LMCS_COUNT]; ///< > RefStruct reference > -- > 2.44.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-request@ffmpeg.org with subject "unsubscribe". >
diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c index 301fa16400..7a8daaa6ad 100644 --- a/libavcodec/vvc/vvc_ps.c +++ b/libavcodec/vvc/vvc_ps.c @@ -219,14 +219,23 @@ fail: return NULL; } -static int decode_sps(VVCParamSets *ps, const H266RawSPS *rsps, void *log_ctx) +static int decode_sps(VVCParamSets *ps, const H266RawSPS *rsps, void *log_ctx, int is_clvss) { const int sps_id = rsps->sps_seq_parameter_set_id; const VVCSPS *old_sps = ps->sps_list[sps_id]; const VVCSPS *sps; - if (old_sps && old_sps->r == rsps) - return 0; + if (is_clvss) { + for (int sps_id = 0; sps_id < VVC_MAX_SPS_COUNT; ++sps_id) + ps->sps_id_used[sps_id] = 0; + } + + if (old_sps) + if (old_sps->r == rsps || !memcmp(old_sps->r, rsps, sizeof(*old_sps->r))) + return 0; + else if (ps->sps_id_used[sps_id]) + return AVERROR_INVALIDDATA; + sps = sps_alloc(rsps, log_ctx); if (!sps) @@ -234,6 +243,7 @@ static int decode_sps(VVCParamSets *ps, const H266RawSPS *rsps, void *log_ctx) ff_refstruct_unref(&ps->sps_list[sps_id]); ps->sps_list[sps_id] = sps; + ps->sps_id_used[sps_id] = 1; return 0; } @@ -610,7 +620,7 @@ static int decode_pps(VVCParamSets *ps, const H266RawPPS *rpps) return ret; } -static int decode_ps(VVCParamSets *ps, const CodedBitstreamH266Context *h266, void *log_ctx) +static int decode_ps(VVCParamSets *ps, const CodedBitstreamH266Context *h266, void *log_ctx, int is_clvss) { const H266RawPictureHeader *ph = h266->ph; const H266RawPPS *rpps; @@ -628,7 +638,7 @@ static int decode_ps(VVCParamSets *ps, const CodedBitstreamH266Context *h266, vo if (!rsps) return AVERROR_INVALIDDATA; - ret = decode_sps(ps, rsps, log_ctx); + ret = decode_sps(ps, rsps, log_ctx, is_clvss); if (ret < 0) return ret; @@ -867,13 +877,16 @@ int ff_vvc_decode_frame_ps(VVCFrameParamSets *fps, struct VVCContext *s) int ret = 0; VVCParamSets *ps = &s->ps; const CodedBitstreamH266Context *h266 = s->cbc->priv_data; + int is_clvss; - ret = decode_ps(ps, h266, s->avctx); + decode_recovery_flag(s); + is_clvss = IS_CLVSS(s); + + ret = decode_ps(ps, h266, s->avctx, is_clvss); if (ret < 0) return ret; - decode_recovery_flag(s); - ret = decode_frame_ps(fps, ps, h266, s->poc_tid0, IS_CLVSS(s)); + ret = decode_frame_ps(fps, ps, h266, s->poc_tid0, is_clvss); decode_recovery_poc(s, &fps->ph); return ret; } diff --git a/libavcodec/vvc/vvc_ps.h b/libavcodec/vvc/vvc_ps.h index f60d8b81c6..b293043de8 100644 --- a/libavcodec/vvc/vvc_ps.h +++ b/libavcodec/vvc/vvc_ps.h @@ -210,6 +210,7 @@ typedef struct VVCLMCS { typedef struct VVCParamSets { const VVCSPS *sps_list[VVC_MAX_SPS_COUNT]; ///< RefStruct reference + int sps_id_used[VVC_MAX_SPS_COUNT]; const VVCPPS *pps_list[VVC_MAX_PPS_COUNT]; ///< RefStruct reference const VVCALF *alf_list[VVC_MAX_ALF_COUNT]; ///< RefStruct reference const H266RawAPS *lmcs_list[VVC_MAX_LMCS_COUNT]; ///< RefStruct reference