diff mbox

[FFmpeg-devel,v2,1/2] lavc/h2645_parse: Don't automatically remove nuh_layer_id > 0 packets

Message ID 20191206031542.20853-1-andriy.gelman@gmail.com
State Accepted
Headers show

Commit Message

Andriy Gelman Dec. 6, 2019, 3:15 a.m. UTC 
From: Andriy Gelman <andriy.gelman@gmail.com>

HEVC standard supports multi-layer streams (ITU-T H.265 02/2018 Annex
F). Each NAL unit belongs to a particular layer defined by nuh_layer_id
in the header.

Currently, all NAL units that do not belong to a base layer are
automatically removed in ff_h2645_packet_split(). Some data may
therefore be lost when future filters/decoders are designed to support
multi-layer streams.

A better approach is to forward nuh_layer_id > 0 packets and let blocks
down the chain decide how to process them. The condition to remove
packets has been moved to hevcdec and cbs.

Found-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
---
 libavcodec/cbs_h2645.c   | 3 +++
 libavcodec/h2645_parse.c | 7 +++----
 libavcodec/h2645_parse.h | 5 +++++
 libavcodec/hevc_parse.c  | 2 ++
 libavcodec/hevc_parser.c | 3 +++
 libavcodec/hevcdec.c     | 2 +-
 6 files changed, 17 insertions(+), 5 deletions(-)

Comments

Andriy Gelman Dec. 14, 2019, 3:18 a.m. UTC | #1 
On Thu, 05. Dec 22:15, Andriy Gelman wrote:
> From: Andriy Gelman <andriy.gelman@gmail.com>
> 
> HEVC standard supports multi-layer streams (ITU-T H.265 02/2018 Annex
> F). Each NAL unit belongs to a particular layer defined by nuh_layer_id
> in the header.
> 
> Currently, all NAL units that do not belong to a base layer are
> automatically removed in ff_h2645_packet_split(). Some data may
> therefore be lost when future filters/decoders are designed to support
> multi-layer streams.
> 
> A better approach is to forward nuh_layer_id > 0 packets and let blocks
> down the chain decide how to process them. The condition to remove
> packets has been moved to hevcdec and cbs.
> 
> Found-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> ---
>  libavcodec/cbs_h2645.c   | 3 +++
>  libavcodec/h2645_parse.c | 7 +++----
>  libavcodec/h2645_parse.h | 5 +++++
>  libavcodec/hevc_parse.c  | 2 ++
>  libavcodec/hevc_parser.c | 3 +++
>  libavcodec/hevcdec.c     | 2 +-
>  6 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 88fa0029cd6..ae32078f246 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -565,6 +565,9 @@ static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
>          AVBufferRef *ref;
>          size_t size = nal->size;
>  
> +        if (nal->nuh_layer_id > 0)
> +            continue;
> +
>          // Remove trailing zeroes.
>          while (size > 0 && nal->data[size - 1] == 0)
>              --size;
> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
> index 4808f79a67f..0f3343004f9 100644
> --- a/libavcodec/h2645_parse.c
> +++ b/libavcodec/h2645_parse.c
> @@ -292,23 +292,22 @@ static int get_bit_length(H2645NAL *nal, int skip_trailing_zeros)
>  static int hevc_parse_nal_header(H2645NAL *nal, void *logctx)
>  {
>      GetBitContext *gb = &nal->gb;
> -    int nuh_layer_id;
>  
>      if (get_bits1(gb) != 0)
>          return AVERROR_INVALIDDATA;
>  
>      nal->type = get_bits(gb, 6);
>  
> -    nuh_layer_id   = get_bits(gb, 6);
> +    nal->nuh_layer_id = get_bits(gb, 6);
>      nal->temporal_id = get_bits(gb, 3) - 1;
>      if (nal->temporal_id < 0)
>          return AVERROR_INVALIDDATA;
>  
>      av_log(logctx, AV_LOG_DEBUG,
>             "nal_unit_type: %d(%s), nuh_layer_id: %d, temporal_id: %d\n",
> -           nal->type, hevc_nal_unit_name(nal->type), nuh_layer_id, nal->temporal_id);
> +           nal->type, hevc_nal_unit_name(nal->type), nal->nuh_layer_id, nal->temporal_id);
>  
> -    return nuh_layer_id == 0;
> +    return 1;
>  }
>  
>  static int h264_parse_nal_header(H2645NAL *nal, void *logctx)
> diff --git a/libavcodec/h2645_parse.h b/libavcodec/h2645_parse.h
> index 2acf882d3da..3e47f86c53b 100644
> --- a/libavcodec/h2645_parse.h
> +++ b/libavcodec/h2645_parse.h
> @@ -56,6 +56,11 @@ typedef struct H2645NAL {
>       */
>      int temporal_id;
>  
> +    /*
> +     * HEVC only, identifier of layer to which nal unit belongs
> +     */
> +    int nuh_layer_id;
> +
>      int skipped_bytes;
>      int skipped_bytes_pos_size;
>      int *skipped_bytes_pos;
> diff --git a/libavcodec/hevc_parse.c b/libavcodec/hevc_parse.c
> index dddb293df64..29dfd479f38 100644
> --- a/libavcodec/hevc_parse.c
> +++ b/libavcodec/hevc_parse.c
> @@ -37,6 +37,8 @@ static int hevc_decode_nal_units(const uint8_t *buf, int buf_size, HEVCParamSets
>  
>      for (i = 0; i < pkt.nb_nals; i++) {
>          H2645NAL *nal = &pkt.nals[i];
> +        if (nal->nuh_layer_id > 0)
> +            continue;
>  
>          /* ignore everything except parameter sets and VCL NALUs */
>          switch (nal->type) {
> diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
> index b444b999550..87d5dba4f5e 100644
> --- a/libavcodec/hevc_parser.c
> +++ b/libavcodec/hevc_parser.c
> @@ -202,6 +202,9 @@ static int parse_nal_units(AVCodecParserContext *s, const uint8_t *buf,
>          H2645NAL *nal = &ctx->pkt.nals[i];
>          GetBitContext *gb = &nal->gb;
>  
> +        if (nal->nuh_layer_id > 0)
> +            continue;
> +
>          switch (nal->type) {
>          case HEVC_NAL_VPS:
>              ff_hevc_decode_nal_vps(gb, avctx, ps);
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index 8f1c162acee..bcd8e67944a 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -3077,7 +3077,7 @@ static int decode_nal_units(HEVCContext *s, const uint8_t *buf, int length)
>  
>          if (s->avctx->skip_frame >= AVDISCARD_ALL ||
>              (s->avctx->skip_frame >= AVDISCARD_NONREF
> -            && ff_hevc_nal_is_nonref(nal->type)))
> +            && ff_hevc_nal_is_nonref(nal->type)) || nal->nuh_layer_id > 0)
>              continue;
>  
>          ret = decode_nal_unit(s, nal);
> -- 
> 2.24.0

ping for both patches

Thanks
Andriy Gelman Jan. 6, 2020, 2:58 a.m. UTC | #2 
On Fri, 13. Dec 22:18, Andriy Gelman wrote:
> On Thu, 05. Dec 22:15, Andriy Gelman wrote:
> > From: Andriy Gelman <andriy.gelman@gmail.com>
> > 
> > HEVC standard supports multi-layer streams (ITU-T H.265 02/2018 Annex
> > F). Each NAL unit belongs to a particular layer defined by nuh_layer_id
> > in the header.
> > 
> > Currently, all NAL units that do not belong to a base layer are
> > automatically removed in ff_h2645_packet_split(). Some data may
> > therefore be lost when future filters/decoders are designed to support
> > multi-layer streams.
> > 
> > A better approach is to forward nuh_layer_id > 0 packets and let blocks
> > down the chain decide how to process them. The condition to remove
> > packets has been moved to hevcdec and cbs.
> > 
> > Found-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> > Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> > ---
> >  libavcodec/cbs_h2645.c   | 3 +++
> >  libavcodec/h2645_parse.c | 7 +++----
> >  libavcodec/h2645_parse.h | 5 +++++
> >  libavcodec/hevc_parse.c  | 2 ++
> >  libavcodec/hevc_parser.c | 3 +++
> >  libavcodec/hevcdec.c     | 2 +-
> >  6 files changed, 17 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> > index 88fa0029cd6..ae32078f246 100644
> > --- a/libavcodec/cbs_h2645.c
> > +++ b/libavcodec/cbs_h2645.c
> > @@ -565,6 +565,9 @@ static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
> >          AVBufferRef *ref;
> >          size_t size = nal->size;
> >  
> > +        if (nal->nuh_layer_id > 0)
> > +            continue;
> > +
> >          // Remove trailing zeroes.
> >          while (size > 0 && nal->data[size - 1] == 0)
> >              --size;
> > diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
> > index 4808f79a67f..0f3343004f9 100644
> > --- a/libavcodec/h2645_parse.c
> > +++ b/libavcodec/h2645_parse.c
> > @@ -292,23 +292,22 @@ static int get_bit_length(H2645NAL *nal, int skip_trailing_zeros)
> >  static int hevc_parse_nal_header(H2645NAL *nal, void *logctx)
> >  {
> >      GetBitContext *gb = &nal->gb;
> > -    int nuh_layer_id;
> >  
> >      if (get_bits1(gb) != 0)
> >          return AVERROR_INVALIDDATA;
> >  
> >      nal->type = get_bits(gb, 6);
> >  
> > -    nuh_layer_id   = get_bits(gb, 6);
> > +    nal->nuh_layer_id = get_bits(gb, 6);
> >      nal->temporal_id = get_bits(gb, 3) - 1;
> >      if (nal->temporal_id < 0)
> >          return AVERROR_INVALIDDATA;
> >  
> >      av_log(logctx, AV_LOG_DEBUG,
> >             "nal_unit_type: %d(%s), nuh_layer_id: %d, temporal_id: %d\n",
> > -           nal->type, hevc_nal_unit_name(nal->type), nuh_layer_id, nal->temporal_id);
> > +           nal->type, hevc_nal_unit_name(nal->type), nal->nuh_layer_id, nal->temporal_id);
> >  
> > -    return nuh_layer_id == 0;
> > +    return 1;
> >  }
> >  
> >  static int h264_parse_nal_header(H2645NAL *nal, void *logctx)
> > diff --git a/libavcodec/h2645_parse.h b/libavcodec/h2645_parse.h
> > index 2acf882d3da..3e47f86c53b 100644
> > --- a/libavcodec/h2645_parse.h
> > +++ b/libavcodec/h2645_parse.h
> > @@ -56,6 +56,11 @@ typedef struct H2645NAL {
> >       */
> >      int temporal_id;
> >  
> > +    /*
> > +     * HEVC only, identifier of layer to which nal unit belongs
> > +     */
> > +    int nuh_layer_id;
> > +
> >      int skipped_bytes;
> >      int skipped_bytes_pos_size;
> >      int *skipped_bytes_pos;
> > diff --git a/libavcodec/hevc_parse.c b/libavcodec/hevc_parse.c
> > index dddb293df64..29dfd479f38 100644
> > --- a/libavcodec/hevc_parse.c
> > +++ b/libavcodec/hevc_parse.c
> > @@ -37,6 +37,8 @@ static int hevc_decode_nal_units(const uint8_t *buf, int buf_size, HEVCParamSets
> >  
> >      for (i = 0; i < pkt.nb_nals; i++) {
> >          H2645NAL *nal = &pkt.nals[i];
> > +        if (nal->nuh_layer_id > 0)
> > +            continue;
> >  
> >          /* ignore everything except parameter sets and VCL NALUs */
> >          switch (nal->type) {
> > diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
> > index b444b999550..87d5dba4f5e 100644
> > --- a/libavcodec/hevc_parser.c
> > +++ b/libavcodec/hevc_parser.c
> > @@ -202,6 +202,9 @@ static int parse_nal_units(AVCodecParserContext *s, const uint8_t *buf,
> >          H2645NAL *nal = &ctx->pkt.nals[i];
> >          GetBitContext *gb = &nal->gb;
> >  
> > +        if (nal->nuh_layer_id > 0)
> > +            continue;
> > +
> >          switch (nal->type) {
> >          case HEVC_NAL_VPS:
> >              ff_hevc_decode_nal_vps(gb, avctx, ps);
> > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> > index 8f1c162acee..bcd8e67944a 100644
> > --- a/libavcodec/hevcdec.c
> > +++ b/libavcodec/hevcdec.c
> > @@ -3077,7 +3077,7 @@ static int decode_nal_units(HEVCContext *s, const uint8_t *buf, int length)
> >  
> >          if (s->avctx->skip_frame >= AVDISCARD_ALL ||
> >              (s->avctx->skip_frame >= AVDISCARD_NONREF
> > -            && ff_hevc_nal_is_nonref(nal->type)))
> > +            && ff_hevc_nal_is_nonref(nal->type)) || nal->nuh_layer_id > 0)
> >              continue;
> >  
> >          ret = decode_nal_unit(s, nal);
> > -- 
> > 2.24.0
> 
> ping for both patches
> 

ping
diff mbox

Patch

diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 88fa0029cd6..ae32078f246 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -565,6 +565,9 @@  static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
         AVBufferRef *ref;
         size_t size = nal->size;
 
+        if (nal->nuh_layer_id > 0)
+            continue;
+
         // Remove trailing zeroes.
         while (size > 0 && nal->data[size - 1] == 0)
             --size;
diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
index 4808f79a67f..0f3343004f9 100644
--- a/libavcodec/h2645_parse.c
+++ b/libavcodec/h2645_parse.c
@@ -292,23 +292,22 @@  static int get_bit_length(H2645NAL *nal, int skip_trailing_zeros)
 static int hevc_parse_nal_header(H2645NAL *nal, void *logctx)
 {
     GetBitContext *gb = &nal->gb;
-    int nuh_layer_id;
 
     if (get_bits1(gb) != 0)
         return AVERROR_INVALIDDATA;
 
     nal->type = get_bits(gb, 6);
 
-    nuh_layer_id   = get_bits(gb, 6);
+    nal->nuh_layer_id = get_bits(gb, 6);
     nal->temporal_id = get_bits(gb, 3) - 1;
     if (nal->temporal_id < 0)
         return AVERROR_INVALIDDATA;
 
     av_log(logctx, AV_LOG_DEBUG,
            "nal_unit_type: %d(%s), nuh_layer_id: %d, temporal_id: %d\n",
-           nal->type, hevc_nal_unit_name(nal->type), nuh_layer_id, nal->temporal_id);
+           nal->type, hevc_nal_unit_name(nal->type), nal->nuh_layer_id, nal->temporal_id);
 
-    return nuh_layer_id == 0;
+    return 1;
 }
 
 static int h264_parse_nal_header(H2645NAL *nal, void *logctx)
diff --git a/libavcodec/h2645_parse.h b/libavcodec/h2645_parse.h
index 2acf882d3da..3e47f86c53b 100644
--- a/libavcodec/h2645_parse.h
+++ b/libavcodec/h2645_parse.h
@@ -56,6 +56,11 @@  typedef struct H2645NAL {
      */
     int temporal_id;
 
+    /*
+     * HEVC only, identifier of layer to which nal unit belongs
+     */
+    int nuh_layer_id;
+
     int skipped_bytes;
     int skipped_bytes_pos_size;
     int *skipped_bytes_pos;
diff --git a/libavcodec/hevc_parse.c b/libavcodec/hevc_parse.c
index dddb293df64..29dfd479f38 100644
--- a/libavcodec/hevc_parse.c
+++ b/libavcodec/hevc_parse.c
@@ -37,6 +37,8 @@  static int hevc_decode_nal_units(const uint8_t *buf, int buf_size, HEVCParamSets
 
     for (i = 0; i < pkt.nb_nals; i++) {
         H2645NAL *nal = &pkt.nals[i];
+        if (nal->nuh_layer_id > 0)
+            continue;
 
         /* ignore everything except parameter sets and VCL NALUs */
         switch (nal->type) {
diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
index b444b999550..87d5dba4f5e 100644
--- a/libavcodec/hevc_parser.c
+++ b/libavcodec/hevc_parser.c
@@ -202,6 +202,9 @@  static int parse_nal_units(AVCodecParserContext *s, const uint8_t *buf,
         H2645NAL *nal = &ctx->pkt.nals[i];
         GetBitContext *gb = &nal->gb;
 
+        if (nal->nuh_layer_id > 0)
+            continue;
+
         switch (nal->type) {
         case HEVC_NAL_VPS:
             ff_hevc_decode_nal_vps(gb, avctx, ps);
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 8f1c162acee..bcd8e67944a 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3077,7 +3077,7 @@  static int decode_nal_units(HEVCContext *s, const uint8_t *buf, int length)
 
         if (s->avctx->skip_frame >= AVDISCARD_ALL ||
             (s->avctx->skip_frame >= AVDISCARD_NONREF
-            && ff_hevc_nal_is_nonref(nal->type)))
+            && ff_hevc_nal_is_nonref(nal->type)) || nal->nuh_layer_id > 0)
             continue;
 
         ret = decode_nal_unit(s, nal);