| Message ID | 20190401094320.10146-1-linjie.fu@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, Apr 01, 2019 at 05:43:20PM +0800, Linjie Fu wrote: > Parse all the constraint flags. > > It can be passed to hw decoders to detemine the exact profile for Range > Extension HEVC. > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavcodec/hevc_ps.c | 18 +++++++++++++++--- > libavcodec/hevc_ps.h | 10 ++++++++++ > 2 files changed, 25 insertions(+), 3 deletions(-) LGTM if this matches the spec thx [...]
On 4/1/2019 6:43 AM, Linjie Fu wrote: > Parse all the constraint flags. > > It can be passed to hw decoders to detemine the exact profile for Range > Extension HEVC. > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavcodec/hevc_ps.c | 18 +++++++++++++++--- > libavcodec/hevc_ps.h | 10 ++++++++++ > 2 files changed, 25 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c > index 80df417e4f..1365a9d640 100644 > --- a/libavcodec/hevc_ps.c > +++ b/libavcodec/hevc_ps.c > @@ -295,9 +295,21 @@ static int decode_profile_tier_level(GetBitContext *gb, AVCodecContext *avctx, > ptl->non_packed_constraint_flag = get_bits1(gb); > ptl->frame_only_constraint_flag = get_bits1(gb); > > - skip_bits(gb, 16); // XXX_reserved_zero_44bits[0..15] > - skip_bits(gb, 16); // XXX_reserved_zero_44bits[16..31] > - skip_bits(gb, 12); // XXX_reserved_zero_44bits[32..43] > + ptl->max_12bit_constraint_flag = get_bits1(gb); > + ptl->max_10bit_constraint_flag = get_bits1(gb); > + ptl->max_8bit_constraint_flag = get_bits1(gb); > + ptl->max_422chroma_constraint_flag = get_bits1(gb); > + ptl->max_420chroma_constraint_flag = get_bits1(gb); > + ptl->max_monochrome_constraint_flag = get_bits1(gb); > + ptl->intra_constraint_flag = get_bits1(gb); > + ptl->one_picture_only_constraint_flag = get_bits1(gb); > + ptl->lower_bit_rate_constraint_flag = get_bits1(gb); > + > + skip_bits(gb, 16); // XXX_reserved_zero_34bits[0..15] The first of these bits can be general_max_14bit_constraint_flag. I suppose no hardware really supports it? > + skip_bits(gb, 16); // XXX_reserved_zero_34bits[16..31] > + skip_bits(gb, 2); // XXX_reserved_zero_34bits[32..33] > + > + ptl->inbld_flag = get_bits1(gb); Strictly speaking, you should be checking ptl->profile_idc and ptl->profile_compatibility_flag to parse these, as per section 7.3.3 in the spec. Despite the amount of bits being fixed and the default value being 0, it would be ideal to prevent misparsing damaged samples. > > return 0; > } > diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h > index bbaa9205ef..1c95a1848b 100644 > --- a/libavcodec/hevc_ps.h > +++ b/libavcodec/hevc_ps.h > @@ -182,6 +182,16 @@ typedef struct PTLCommon { > uint8_t interlaced_source_flag; > uint8_t non_packed_constraint_flag; > uint8_t frame_only_constraint_flag; > + uint8_t max_12bit_constraint_flag; > + uint8_t max_10bit_constraint_flag; > + uint8_t max_8bit_constraint_flag; > + uint8_t max_422chroma_constraint_flag; > + uint8_t max_420chroma_constraint_flag; > + uint8_t max_monochrome_constraint_flag; > + uint8_t intra_constraint_flag; > + uint8_t one_picture_only_constraint_flag; > + uint8_t lower_bit_rate_constraint_flag; > + uint8_t inbld_flag; > } PTLCommon; > > typedef struct PTL { >
On 01/04/2019 22:38, James Almer wrote: > On 4/1/2019 6:43 AM, Linjie Fu wrote: >> Parse all the constraint flags. >> >> It can be passed to hw decoders to detemine the exact profile for Range >> Extension HEVC. >> >> Signed-off-by: Linjie Fu <linjie.fu@intel.com> >> --- >> libavcodec/hevc_ps.c | 18 +++++++++++++++--- >> libavcodec/hevc_ps.h | 10 ++++++++++ >> 2 files changed, 25 insertions(+), 3 deletions(-) >> >> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c >> index 80df417e4f..1365a9d640 100644 >> --- a/libavcodec/hevc_ps.c >> +++ b/libavcodec/hevc_ps.c >> @@ -295,9 +295,21 @@ static int decode_profile_tier_level(GetBitContext *gb, AVCodecContext *avctx, >> ptl->non_packed_constraint_flag = get_bits1(gb); >> ptl->frame_only_constraint_flag = get_bits1(gb); >> >> - skip_bits(gb, 16); // XXX_reserved_zero_44bits[0..15] >> - skip_bits(gb, 16); // XXX_reserved_zero_44bits[16..31] >> - skip_bits(gb, 12); // XXX_reserved_zero_44bits[32..43] >> + ptl->max_12bit_constraint_flag = get_bits1(gb); >> + ptl->max_10bit_constraint_flag = get_bits1(gb); >> + ptl->max_8bit_constraint_flag = get_bits1(gb); >> + ptl->max_422chroma_constraint_flag = get_bits1(gb); >> + ptl->max_420chroma_constraint_flag = get_bits1(gb); >> + ptl->max_monochrome_constraint_flag = get_bits1(gb); >> + ptl->intra_constraint_flag = get_bits1(gb); >> + ptl->one_picture_only_constraint_flag = get_bits1(gb); >> + ptl->lower_bit_rate_constraint_flag = get_bits1(gb); >> + >> + skip_bits(gb, 16); // XXX_reserved_zero_34bits[0..15] > > The first of these bits can be general_max_14bit_constraint_flag. I > suppose no hardware really supports it? I think it's worth getting these right anyway. From the set of flags you've ended up with here I'm guessing you're working from a pre-2018 version of the standard? Try the 201802 version - <https://www.itu.int/rec/T-REC-H.265-201802-I/en>. >> + skip_bits(gb, 16); // XXX_reserved_zero_34bits[16..31] >> + skip_bits(gb, 2); // XXX_reserved_zero_34bits[32..33] >> + >> + ptl->inbld_flag = get_bits1(gb); > > Strictly speaking, you should be checking ptl->profile_idc and > ptl->profile_compatibility_flag to parse these, as per section 7.3.3 in > the spec. > Despite the amount of bits being fixed and the default value being 0, it > would be ideal to prevent misparsing damaged samples. +1. Future profiles which could assign completely different meanings to these bits are also a possibility, though I would hope the standard people would avoid doing that. Thanks, - Mark
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Tuesday, April 2, 2019 06:24 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_ps: parse constraint flags for > HEVC REXT > > On 01/04/2019 22:38, James Almer wrote: > > On 4/1/2019 6:43 AM, Linjie Fu wrote: > >> Parse all the constraint flags. > >> > >> It can be passed to hw decoders to detemine the exact profile for Range > >> Extension HEVC. > >> > >> Signed-off-by: Linjie Fu <linjie.fu@intel.com> > >> --- > >> libavcodec/hevc_ps.c | 18 +++++++++++++++--- > >> libavcodec/hevc_ps.h | 10 ++++++++++ > >> 2 files changed, 25 insertions(+), 3 deletions(-) > >> > >> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c > >> index 80df417e4f..1365a9d640 100644 > >> --- a/libavcodec/hevc_ps.c > >> +++ b/libavcodec/hevc_ps.c > >> @@ -295,9 +295,21 @@ static int > decode_profile_tier_level(GetBitContext *gb, AVCodecContext *avctx, > >> ptl->non_packed_constraint_flag = get_bits1(gb); > >> ptl->frame_only_constraint_flag = get_bits1(gb); > >> > >> - skip_bits(gb, 16); // XXX_reserved_zero_44bits[0..15] > >> - skip_bits(gb, 16); // XXX_reserved_zero_44bits[16..31] > >> - skip_bits(gb, 12); // XXX_reserved_zero_44bits[32..43] > >> + ptl->max_12bit_constraint_flag = get_bits1(gb); > >> + ptl->max_10bit_constraint_flag = get_bits1(gb); > >> + ptl->max_8bit_constraint_flag = get_bits1(gb); > >> + ptl->max_422chroma_constraint_flag = get_bits1(gb); > >> + ptl->max_420chroma_constraint_flag = get_bits1(gb); > >> + ptl->max_monochrome_constraint_flag = get_bits1(gb); > >> + ptl->intra_constraint_flag = get_bits1(gb); > >> + ptl->one_picture_only_constraint_flag = get_bits1(gb); > >> + ptl->lower_bit_rate_constraint_flag = get_bits1(gb); > >> + > >> + skip_bits(gb, 16); // XXX_reserved_zero_34bits[0..15] > > > > The first of these bits can be general_max_14bit_constraint_flag. I > > suppose no hardware really supports it? > > I think it's worth getting these right anyway. > > From the set of flags you've ended up with here I'm guessing you're working > from a pre-2018 version of the standard? Try the 201802 version - > <https://www.itu.int/rec/T-REC-H.265-201802-I/en>. > > >> + skip_bits(gb, 16); // XXX_reserved_zero_34bits[16..31] > >> + skip_bits(gb, 2); // XXX_reserved_zero_34bits[32..33] > >> + > >> + ptl->inbld_flag = get_bits1(gb); > > > > Strictly speaking, you should be checking ptl->profile_idc and > > ptl->profile_compatibility_flag to parse these, as per section 7.3.3 in > > the spec. > > Despite the amount of bits being fixed and the default value being 0, it > > would be ideal to prevent misparsing damaged samples. > > +1. Future profiles which could assign completely different meanings to > these bits are also a possibility, though I would hope the standard people > would avoid doing that. Thanks for the suggestions, refined patch has been sent.
diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c index 80df417e4f..1365a9d640 100644 --- a/libavcodec/hevc_ps.c +++ b/libavcodec/hevc_ps.c @@ -295,9 +295,21 @@ static int decode_profile_tier_level(GetBitContext *gb, AVCodecContext *avctx, ptl->non_packed_constraint_flag = get_bits1(gb); ptl->frame_only_constraint_flag = get_bits1(gb); - skip_bits(gb, 16); // XXX_reserved_zero_44bits[0..15] - skip_bits(gb, 16); // XXX_reserved_zero_44bits[16..31] - skip_bits(gb, 12); // XXX_reserved_zero_44bits[32..43] + ptl->max_12bit_constraint_flag = get_bits1(gb); + ptl->max_10bit_constraint_flag = get_bits1(gb); + ptl->max_8bit_constraint_flag = get_bits1(gb); + ptl->max_422chroma_constraint_flag = get_bits1(gb); + ptl->max_420chroma_constraint_flag = get_bits1(gb); + ptl->max_monochrome_constraint_flag = get_bits1(gb); + ptl->intra_constraint_flag = get_bits1(gb); + ptl->one_picture_only_constraint_flag = get_bits1(gb); + ptl->lower_bit_rate_constraint_flag = get_bits1(gb); + + skip_bits(gb, 16); // XXX_reserved_zero_34bits[0..15] + skip_bits(gb, 16); // XXX_reserved_zero_34bits[16..31] + skip_bits(gb, 2); // XXX_reserved_zero_34bits[32..33] + + ptl->inbld_flag = get_bits1(gb); return 0; } diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h index bbaa9205ef..1c95a1848b 100644 --- a/libavcodec/hevc_ps.h +++ b/libavcodec/hevc_ps.h @@ -182,6 +182,16 @@ typedef struct PTLCommon { uint8_t interlaced_source_flag; uint8_t non_packed_constraint_flag; uint8_t frame_only_constraint_flag; + uint8_t max_12bit_constraint_flag; + uint8_t max_10bit_constraint_flag; + uint8_t max_8bit_constraint_flag; + uint8_t max_422chroma_constraint_flag; + uint8_t max_420chroma_constraint_flag; + uint8_t max_monochrome_constraint_flag; + uint8_t intra_constraint_flag; + uint8_t one_picture_only_constraint_flag; + uint8_t lower_bit_rate_constraint_flag; + uint8_t inbld_flag; } PTLCommon; typedef struct PTL {
Parse all the constraint flags. It can be passed to hw decoders to detemine the exact profile for Range Extension HEVC. Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- libavcodec/hevc_ps.c | 18 +++++++++++++++--- libavcodec/hevc_ps.h | 10 ++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-)