On Sun, 11 Oct 2015 21:55:12 +0200 Michael Niedermayer <mich...@niedermayer.cc> wrote:
> On Sun, Oct 11, 2015 at 09:39:32PM +0200, wm4 wrote: > > On Sun, 11 Oct 2015 21:16:27 +0200 > > Michael Niedermayer <michae...@gmx.at> wrote: > > > > > From: Michael Niedermayer <mich...@niedermayer.cc> > > > > > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > > --- > > > libavcodec/h264_slice.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c > > > index cce97d9..daa3737 100644 > > > --- a/libavcodec/h264_slice.c > > > +++ b/libavcodec/h264_slice.c > > > @@ -985,6 +985,10 @@ static enum AVPixelFormat > > > get_pixel_format(H264Context *h, int force_callback) > > > for (i=0; choices[i] != AV_PIX_FMT_NONE; i++) > > > if (non_j_pixfmt(choices[i]) == non_j_pixfmt(h->avctx->pix_fmt) > > > && !force_callback) > > > return choices[i]; > > > + > > > + if (!force_callback) > > > + return AV_PIX_FMT_NONE; > > > + > > > return ff_thread_get_format(h->avctx, choices); > > > } > > > > > > > So if I can see this right, the whole purpose of this is to check > > whether the h264 parameters map to a lavc pixfmt, and bail out or > > reinitialize if it doesn't. Why can't this be delayed later? What > > actually needs it sooner than the first "real" get_format? For dealing > > > with paramater changes, why can't it check the raw parameters instead? > > The raw parameters are checked as well but iam not sure these checks > are complete, they are more complex. > I've checked again. 3 parameters influence the pixfmt: bit_depth_luma, chroma_format_idc, and colorspace. Maybe add color_range because of the J formats. The first two are already checked (lazily, if the SPS changes). A colorspace change also triggers reinit, although the check for it is buried somewhat deeper in the code. (Also I see a potential bug there: are colorspace and other fields not reset correctly if the SPS changes, and the new SPS doesn't have these fields set anymore?) A changing color_range does not force reinit; this must be fixed (although I'd prefer dropping the J hack completely). So do you agree that checks for colorspace and color_range should be added, and that they should trigger reinit, and that this combined with my patch would fix all the potential issues the patch could introduce? Note that because of hwaccels, get_format should always be triggered if the SPS changes in any way, because some hwaccels might rely on the current SPS information. I'm also questioning why there are so many "clever" fine grained reinit checks. Wouldn't it be better to fully reinit every time there is a new SPS, and the new SPS is different than the previous SPS on the byte level? (Don't worry, I'm only speaking hypothetically.) _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
