On Thu, 2018-05-24 at 10:07 +0200, Hendrik Leppkes wrote:
> On Thu, May 24, 2018 at 8:22 AM, Xiang, Haihao <haihao.xi...@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.xi...@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.xi...@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@inte
> > > > > > > l.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@in
> > > > > > > > > tel.
> > > > > > > > > com>
> > > > > > > > > wrote:
> > > > > > > > > > Signed-off-by: Haihao Xiang <haihao.xi...@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.htm
> > > > > > > > > l
> > > > > > > > >
> > > > > > > >
> > > > > > > > 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_offs
> > > > et/
> > > > 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.
>
If so, it seems there are lots of interpretation in sps. For example, the checks
below are used for colour properties in sps:
// Set invalid values to "unspecified"
if (!av_color_primaries_name(vui->colour_primaries))
vui->colour_primaries = AVCOL_PRI_UNSPECIFIED;
if (!av_color_transfer_name(vui->transfer_characteristic))
vui->transfer_characteristic = AVCOL_TRC_UNSPECIFIED;
if (!av_color_space_name(vui->matrix_coeffs))
vui->matrix_coeffs = AVCOL_SPC_UNSPECIFIED;
Do you think we should remove the above code from sps? I think it will be
inconvenient to use the colour properties if we don't check the values in sps.
Thanks
Haihao
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
