On Sun, Oct 11, 2015 at 10:43:04PM +0200, wm4 wrote: > 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.)
i suspect that would break some file somewhere somehow but iam happy with anything that works [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel