Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] lavc/vvc: Error if SPS ID is duplicated within CVS
@ 2024-04-03 19:01 Frank Plowman
  2024-04-06 12:52 ` Nuo Mi
  0 siblings, 1 reply; 2+ messages in thread
From: Frank Plowman @ 2024-04-03 19:01 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Frank Plowman

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)
+        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
-- 
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".

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [FFmpeg-devel] [PATCH] lavc/vvc: Error if SPS ID is duplicated within CVS
  2024-04-03 19:01 [FFmpeg-devel] [PATCH] lavc/vvc: Error if SPS ID is duplicated within CVS Frank Plowman
@ 2024-04-06 12:52 ` Nuo Mi
  0 siblings, 0 replies; 2+ messages in thread
From: Nuo Mi @ 2024-04-06 12:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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".
>
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-04-06 12:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-03 19:01 [FFmpeg-devel] [PATCH] lavc/vvc: Error if SPS ID is duplicated within CVS Frank Plowman
2024-04-06 12:52 ` Nuo Mi

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git