On 16 March 2016 at 23:10, Hendrik Leppkes <h.lepp...@gmail.com> wrote:
> On Wed, Mar 16, 2016 at 1:04 PM, wm4 <nfx...@googlemail.com> wrote: > > On Wed, 16 Mar 2016 22:55:09 +1100 > > Matt Oliver <protogo...@gmail.com> wrote: > > > >> On 16 March 2016 at 22:32, Clément Bœsch <u...@pkh.me> wrote: > >> > >> > On Wed, Mar 16, 2016 at 12:31:35PM +0100, Matt Oliver wrote: > >> > > ffmpeg | branch: master | Matt Oliver <protogo...@gmail.com> | Wed > Mar > >> > 16 22:28:29 2016 +1100| [109dfed7fc265f3e071854d5e6de5dd7f82ff9fb] | > >> > committer: Matt Oliver > >> > > > >> > > lavc/dxva2_h264: Fix incorrect assert statement. > >> > > > >> > > Signed-off-by: Matt Oliver <protogo...@gmail.com> > >> > > > >> > > > > >> > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=109dfed7fc265f3e071854d5e6de5dd7f82ff9fb > >> > > --- > >> > > > >> > > libavcodec/dxva2_h264.c | 7 ++++++- > >> > > 1 file changed, 6 insertions(+), 1 deletion(-) > >> > > > >> > > diff --git a/libavcodec/dxva2_h264.c b/libavcodec/dxva2_h264.c > >> > > index 61cce3a..54f2b80 100644 > >> > > --- a/libavcodec/dxva2_h264.c > >> > > +++ b/libavcodec/dxva2_h264.c > >> > > @@ -426,7 +426,12 @@ static int > >> > commit_bitstream_and_slice_buffer(AVCodecContext *avctx, > >> > > slice_data = ctx_pic->slice_long; > >> > > slice_size = ctx_pic->slice_count * > >> > sizeof(*ctx_pic->slice_long); > >> > > } > >> > > - assert((bs->DataSize & 127) == 0); > >> > > +#if CONFIG_D3D11VA > >> > > + assert((((D3D11_VIDEO_DECODER_BUFFER_DESC *)bs)->DataSize & > 127) == > >> > 0); > >> > > +#endif > >> > > +#if CONFIG_DXVA2 > >> > > + assert((((DXVA2_DecodeBufferDesc *)bs)->DataSize & 127) == 0); > >> > > +#endif > >> > > >> > please use av_assert* > >> > >> > >> My apologies, I just modified the existing assert usage so that it > doesnt > >> generate a compile error (which I thought was a rather trivial patch). > >> There are other uses of assert instead of av_assert in the same file, do > >> you want an additional patch that changes all uses (although im sure > there > >> are many other locations that use assert still) > > > > In addition to this, is this even correct? Both D3D11VA and DXVA2 can > > be defined AFAIK. > > This patch is indeed not correct. Please submit patches for review on > the ML in the future for components you do not maintain. My bad. The previous code did not compile as type for bs was a typedef for void* and so there is no DataSize element. This has been broken for a while without anyone realising as noone tested a debug build. I missed adding the pixel format conditionals as I made the assumption that if these were not true then given that this function is only ever called from ff_dxva2_common_end_frame and if neither of those conditionals are met then both bs and sc would be passed as NULL. I made the assumption given that the original assert in the original code didnt check for possibility of being NULL that this was therefore originally considered to not be an issue. Obviously I gave the original code a bit to much credit. So the patch was more about fixing a compilation error in the code rather than fixing that and an existing logic error that I didnt see. So your right I missed the other half, mea culpa, thanks Hendrik for fixing the logic error as well. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
