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

Submitted by Andriy Gelman on Dec. 2, 2019, 2:48 a.m.

Details

Message ID 20191202024853.13412-1-andriy.gelman@gmail.com
State New
Headers show

Commit Message

Andriy Gelman Dec. 2, 2019, 2:48 a.m.
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 bsf where such packets are
currently not supported.
---
 libavcodec/cbs_h2645.c   | 3 +++
 libavcodec/h2645_parse.c | 7 +++----
 libavcodec/h2645_parse.h | 5 +++++
 libavcodec/hevc_parse.c  | 2 ++
 libavcodec/hevc_parser.c | 2 ++
 libavcodec/hevcdec.c     | 2 +-
 6 files changed, 16 insertions(+), 5 deletions(-)

Comments

James Almer Dec. 2, 2019, 3:07 a.m.
On 12/1/2019 11:48 PM, 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 bsf where such packets are
> currently not supported.
> ---
>  libavcodec/cbs_h2645.c   | 3 +++
>  libavcodec/h2645_parse.c | 7 +++----
>  libavcodec/h2645_parse.h | 5 +++++
>  libavcodec/hevc_parse.c  | 2 ++
>  libavcodec/hevc_parser.c | 2 ++
>  libavcodec/hevcdec.c     | 2 +-
>  6 files changed, 16 insertions(+), 5 deletions(-)

Missing changes to extract_extradata_bsf.

> 
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 88fa0029cd6..9f89f1c5a55 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -562,6 +562,9 @@ static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
>  
>      for (i = 0; i < packet->nb_nals; i++) {
>          const H2645NAL *nal = &packet->nals[i];
> +        if (nal->nuh_layer_id > 0)
> +            continue;

CBS itself must not ignore them. Users of CBS should be able to choose
to ignore them, same way you're doing it for h2645_parse and its users
below.

> +
>          AVBufferRef *ref;
>          size_t size = nal->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..a47104aea9c 100644
> --- a/libavcodec/hevc_parser.c
> +++ b/libavcodec/hevc_parser.c
> @@ -200,6 +200,8 @@ static int parse_nal_units(AVCodecParserContext *s, const uint8_t *buf,
>  
>      for (i = 0; i < ctx->pkt.nb_nals; i++) {
>          H2645NAL *nal = &ctx->pkt.nals[i];
> +        if (nal->nuh_layer_id > 0)
> +            continue;
>          GetBitContext *gb = &nal->gb;
>  
>          switch (nal->type) {
> 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);
>
James Almer Dec. 2, 2019, 3:10 a.m.
On 12/2/2019 12:07 AM, James Almer wrote:
> On 12/1/2019 11:48 PM, 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 bsf where such packets are
>> currently not supported.
>> ---
>>  libavcodec/cbs_h2645.c   | 3 +++
>>  libavcodec/h2645_parse.c | 7 +++----
>>  libavcodec/h2645_parse.h | 5 +++++
>>  libavcodec/hevc_parse.c  | 2 ++
>>  libavcodec/hevc_parser.c | 2 ++
>>  libavcodec/hevcdec.c     | 2 +-
>>  6 files changed, 16 insertions(+), 5 deletions(-)
> 
> Missing changes to extract_extradata_bsf.

Actually, probably not. Skipping them there may result in lost data in
the output packet.
Andriy Gelman Dec. 2, 2019, 3:30 a.m.
On Mon, 02. Dec 00:07, James Almer wrote:
> On 12/1/2019 11:48 PM, 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 bsf where such packets are
> > currently not supported.
> > ---
> >  libavcodec/cbs_h2645.c   | 3 +++
> >  libavcodec/h2645_parse.c | 7 +++----
> >  libavcodec/h2645_parse.h | 5 +++++
> >  libavcodec/hevc_parse.c  | 2 ++
> >  libavcodec/hevc_parser.c | 2 ++
> >  libavcodec/hevcdec.c     | 2 +-
> >  6 files changed, 16 insertions(+), 5 deletions(-)
> 
> Missing changes to extract_extradata_bsf.

I did look into this. My reasoning for not modifying extract_extractdata is because
the filter doesn't parse parameter sets and only makes them available in packet side
data. 

> 
> > 
> > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> > index 88fa0029cd6..9f89f1c5a55 100644
> > --- a/libavcodec/cbs_h2645.c
> > +++ b/libavcodec/cbs_h2645.c
> > @@ -562,6 +562,9 @@ static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
> >  
> >      for (i = 0; i < packet->nb_nals; i++) {
> >          const H2645NAL *nal = &packet->nals[i];
> > +        if (nal->nuh_layer_id > 0)
> > +            continue;
> 
> CBS itself must not ignore them. Users of CBS should be able to choose
> to ignore them, same way you're doing it for h2645_parse and its users
> below.

ok, will update.

Thanks
Andreas Rheinhardt Dec. 2, 2019, 6:50 a.m.
On Mon, Dec 2, 2019 at 3:49 AM Andriy Gelman <andriy.gelman@gmail.com>
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 bsf where such packets are
> currently not supported.
> ---
>  libavcodec/cbs_h2645.c   | 3 +++
>  libavcodec/h2645_parse.c | 7 +++----
>  libavcodec/h2645_parse.h | 5 +++++
>  libavcodec/hevc_parse.c  | 2 ++
>  libavcodec/hevc_parser.c | 2 ++
>  libavcodec/hevcdec.c     | 2 +-
>  6 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 88fa0029cd6..9f89f1c5a55 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -562,6 +562,9 @@ static int
> cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
>
>      for (i = 0; i < packet->nb_nals; i++) {
>          const H2645NAL *nal = &packet->nals[i];
> +        if (nal->nuh_layer_id > 0)
> +            continue;
> +
>

Mixed declaration and code. But, as James has already said: cbs should not
drop higher layers by default.


>          AVBufferRef *ref;
>          size_t size = nal->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;
> +
>

You might want to add a commit on top of that to reorder the H2645NAL
entries so that the size doesn't increase.


>      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..a47104aea9c 100644
> --- a/libavcodec/hevc_parser.c
> +++ b/libavcodec/hevc_parser.c
> @@ -200,6 +200,8 @@ static int parse_nal_units(AVCodecParserContext *s,
> const uint8_t *buf,
>
>      for (i = 0; i < ctx->pkt.nb_nals; i++) {
>          H2645NAL *nal = &ctx->pkt.nals[i];
> +        if (nal->nuh_layer_id > 0)
> +            continue;
>

Mixed declaration and code.


>          GetBitContext *gb = &nal->gb;
>
>          switch (nal->type) {
> 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
>
> _______________________________________________
> 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".
Andriy Gelman Dec. 2, 2019, 2:30 p.m.
On Mon, 02. Dec 07:50, Andreas Rheinhardt wrote:
> On Mon, Dec 2, 2019 at 3:49 AM Andriy Gelman <andriy.gelman@gmail.com>
> 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 bsf where such packets are
> > currently not supported.
> > ---
> >  libavcodec/cbs_h2645.c   | 3 +++
> >  libavcodec/h2645_parse.c | 7 +++----
> >  libavcodec/h2645_parse.h | 5 +++++
> >  libavcodec/hevc_parse.c  | 2 ++
> >  libavcodec/hevc_parser.c | 2 ++
> >  libavcodec/hevcdec.c     | 2 +-
> >  6 files changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> > index 88fa0029cd6..9f89f1c5a55 100644
> > --- a/libavcodec/cbs_h2645.c
> > +++ b/libavcodec/cbs_h2645.c
> > @@ -562,6 +562,9 @@ static int
> > cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
> >
> >      for (i = 0; i < packet->nb_nals; i++) {
> >          const H2645NAL *nal = &packet->nals[i];
> > +        if (nal->nuh_layer_id > 0)
> > +            continue;
> > +
> >
> 
> Mixed declaration and code. But, as James has already said: cbs should not
> drop higher layers by default.
> 
> 
> >          AVBufferRef *ref;
> >          size_t size = nal->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;
> > +
> >
> 
> You might want to add a commit on top of that to reorder the H2645NAL
> entries so that the size doesn't increase.
> 

Thanks for looking over patch. 
I'll update and resend.
Andriy Gelman Dec. 5, 2019, 9:35 p.m.
On Mon, 02. Dec 00:07, James Almer wrote:
> On 12/1/2019 11:48 PM, 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 bsf where such packets are
> > currently not supported.
> > ---
> >  libavcodec/cbs_h2645.c   | 3 +++
> >  libavcodec/h2645_parse.c | 7 +++----
> >  libavcodec/h2645_parse.h | 5 +++++
> >  libavcodec/hevc_parse.c  | 2 ++
> >  libavcodec/hevc_parser.c | 2 ++
> >  libavcodec/hevcdec.c     | 2 +-
> >  6 files changed, 16 insertions(+), 5 deletions(-)
> 
> Missing changes to extract_extradata_bsf.
> 
> > 
> > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> > index 88fa0029cd6..9f89f1c5a55 100644
> > --- a/libavcodec/cbs_h2645.c
> > +++ b/libavcodec/cbs_h2645.c

> > @@ -562,6 +562,9 @@ static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
> >  
> >      for (i = 0; i < packet->nb_nals; i++) {
> >          const H2645NAL *nal = &packet->nals[i];
> > +        if (nal->nuh_layer_id > 0)
> > +            continue;
> 
> CBS itself must not ignore them. Users of CBS should be able to choose
> to ignore them, same way you're doing it for h2645_parse and its users
> below.

James, Andreas,

The parameter sets for multi-layer streams are parsed differently to the
base-layer. See for example SPS p.35 vs p.432 ITU-H265 (02/2018). 

CBS only supports parsing nuh_layer_id = 0 at the moment. I have checked with a
multi-layer stream and CBS errors out. 

Do you agree then that the above check should stay in the patch? 

Thanks,

Patch hide | download patch | download mbox

diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 88fa0029cd6..9f89f1c5a55 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -562,6 +562,9 @@  static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
 
     for (i = 0; i < packet->nb_nals; i++) {
         const H2645NAL *nal = &packet->nals[i];
+        if (nal->nuh_layer_id > 0)
+            continue;
+
         AVBufferRef *ref;
         size_t size = nal->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..a47104aea9c 100644
--- a/libavcodec/hevc_parser.c
+++ b/libavcodec/hevc_parser.c
@@ -200,6 +200,8 @@  static int parse_nal_units(AVCodecParserContext *s, const uint8_t *buf,
 
     for (i = 0; i < ctx->pkt.nb_nals; i++) {
         H2645NAL *nal = &ctx->pkt.nals[i];
+        if (nal->nuh_layer_id > 0)
+            continue;
         GetBitContext *gb = &nal->gb;
 
         switch (nal->type) {
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);