[FFmpeg-devel] [PATCH 1/2] hevcdec: Miss the location of chroma samples when exporting stream parameters

Hendrik Leppkes h.leppkes at gmail.com
Thu May 24 11:07:19 EEST 2018


On Thu, May 24, 2018 at 8:22 AM, Xiang, Haihao <haihao.xiang at intel.com> wrote:
> On Fri, 2018-05-18 at 11:13 +0200, Hendrik Leppkes wrote:
>> On Fri, May 18, 2018 at 10:52 AM, Xiang, Haihao <haihao.xiang at intel.com>
>> wrote:
>> > On Thu, 2018-05-17 at 12:30 +0200, Hendrik Leppkes wrote:
>> > > On Thu, May 17, 2018 at 8:08 AM, Xiang, Haihao <haihao.xiang at intel.com>
>> > > wrote:
>> > > > On Wed, 2018-05-16 at 11:27 +0200, Hendrik Leppkes wrote:
>> > > > > On Wed, May 16, 2018 at 10:49 AM, Xiang, Haihao <haihao.xiang at intel.co
>> > > > > m>
>> > > > > wrote:
>> > > > > > On Wed, 2018-05-16 at 10:17 +0200, Hendrik Leppkes wrote:
>> > > > > > > On Wed, May 16, 2018 at 9:19 AM, Haihao Xiang <haihao.xiang at intel.
>> > > > > > > com>
>> > > > > > > wrote:
>> > > > > > > > Signed-off-by: Haihao Xiang <haihao.xiang at intel.com>
>> > > > > > > > ---
>> > > > > > > >  libavcodec/hevcdec.c | 5 +++++
>> > > > > > > >  1 file changed, 5 insertions(+)
>> > > > > > > >
>> > > > > > > > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
>> > > > > > > > index c8877626d2..13d868bb4f 100644
>> > > > > > > > --- a/libavcodec/hevcdec.c
>> > > > > > > > +++ b/libavcodec/hevcdec.c
>> > > > > > > > @@ -344,6 +344,11 @@ static void
>> > > > > > > > export_stream_params(AVCodecContext
>> > > > > > > > *avctx,
>> > > > > > > > const HEVCParamSets *ps,
>> > > > > > > >          avctx->colorspace      = AVCOL_SPC_UNSPECIFIED;
>> > > > > > > >      }
>> > > > > > > >
>> > > > > > > > +    if (sps->vui.chroma_loc_info_present_flag)
>> > > > > > > > +        avctx->chroma_sample_location = sps-
>> > > > > > > > > vui.chroma_sample_loc_type_top_field + 1;
>> > > > > > > >
>> > > > > > > > +    else
>> > > > > > > > +        avctx->chroma_sample_location =
>> > > > > > > > AVCHROMA_LOC_UNSPECIFIED;
>> > > > > > > >
>> > > > > > >
>> > > > > > > This change is incomplete, refer to the patch I proposed earlier:
>> > > > > > > http://ffmpeg.org/pipermail/ffmpeg-devel/2018-April/228100.html
>> > > > > > >
>> > > > > >
>> > > > > > Sorry I didn't see your proposal before. Per spec, once
>> > > > > > chroma_loc_info_present_flag is set, chroma_format_idc should be 1.
>> > > > > > I
>> > > > > > think
>> > > > > > it
>> > > > > > would be better to add some checks when parsing the sequence
>> > > > > > parameters.
>> > > > > >
>> > > > >
>> > > > > It makes no real difference because you still need the check to be
>> > > > > able to set the LEFT default value for  4:2:0 only.
>> > > >
>> > > > If chroma_sample_loc_type_top_field is set correctly when parsing SPS,
>> > > > we
>> > > > may
>> > > > set chroma_sample_loc_type_top_field + 1 for 4:2:0 below no matter the
>> > > > present
>> > > > flag
>> > > >
>> > > > if (sps->chroma_format_idc == 1)
>> > > >     avctx->chroma_sample_location = sps-
>> > > > > vui.chroma_sample_loc_type_top_field +
>> > > >
>> > > > 1;
>> > > > else
>> > > >     avctx->chroma_sample_location = AVCHROMA_LOC_UNSPECIFIED;
>> > > >
>> > >
>> > > The fields in the sps struct should reflect the bitstream,
>> > > interpretation should be left out of it.
>> > >
>> >
>> > Actually the checks for some field are done in sps, e.g.
>> > def_disp_win_left_offset/def_disp_win_right_offset/def_disp_win_top_offset/
>> > def_disp_win_bottom_offset. User needn't worry about the values when using
>> > these
>> > fields.
>> >
>> >
>>
>> These fields are not available to "users".
>
> Per the current implementation, sps->vui.def_disp_win may impact avctx-
>>width/avctx->height, so I mean these fields may be used.
>
> BTW setting sps->vui.chroma_sample_loc_type_top_field to 0 for 4:2:0 in sps
> structure should reflect the bitstream, why do you think it is interpretation?
>

Is the value coded in the bitstream? If not, then its interpretation.

- Hendrik


More information about the ffmpeg-devel mailing list