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.xi...@intel.com> > > > 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.xi...@intel.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.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. > Like mentioned above, you always need the same checks anyway, all you > do is move it into another place. It is IMHO much cleaner to keep the > interpretation of all those values in the export_stream_params > function, its not like its complex or anything, but it cleanly > seperates parsing and interpretation, and ensures the sps struct only > contains values directly read from the bitstream. > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
