diff mbox

[FFmpeg-devel,4/6] h264: don't re-assign H264Picture::mbaff between slices.

Message ID 1490796744-76454-4-git-send-email-rsbultje@gmail.com
State New
Headers show

Commit Message

Ronald S. Bultje March 29, 2017, 2:12 p.m. UTC
The value must be identical between slices. (Maybe this needs an error
path to inform caller if this fails?)
---
 libavcodec/h264_direct.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer March 30, 2017, 12:06 a.m. UTC | #1
On Wed, Mar 29, 2017 at 10:12:22AM -0400, Ronald S. Bultje wrote:
> The value must be identical between slices. (Maybe this needs an error
> path to inform caller if this fails?)
> ---
>  libavcodec/h264_direct.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/h264_direct.c b/libavcodec/h264_direct.c
> index 4e7202b..20136b0 100644
> --- a/libavcodec/h264_direct.c
> +++ b/libavcodec/h264_direct.c
> @@ -138,7 +138,11 @@ void ff_h264_direct_ref_list_init(const H264Context *const h, H264SliceContext *
>          memcpy(cur->ref_poc[1],   cur->ref_poc[0],   sizeof(cur->ref_poc[0]));
>      }
>  
> -    cur->mbaff = FRAME_MBAFF(h);
> +    if (h->current_slice == 0) {
> +        cur->mbaff = FRAME_MBAFF(h);
> +    } else if (cur->mbaff != FRAME_MBAFF(h)) {
> +        av_log(h->avctx, AV_LOG_ERROR, "Frame mixes MBAFF and non-MBAFF slices\n");
> +    }

It should not be possible for mbaff to differ between slices without
some checks failing
we check for sps and frame/field to match thogh this code was shuffled
around with some merges i think it still works

so maybe a assert0 would work for this

thx

[...]
Ronald S. Bultje March 30, 2017, 11:49 a.m. UTC | #2
Hi,

On Wed, Mar 29, 2017 at 8:06 PM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Wed, Mar 29, 2017 at 10:12:22AM -0400, Ronald S. Bultje wrote:
> > The value must be identical between slices. (Maybe this needs an error
> > path to inform caller if this fails?)
> > ---
> >  libavcodec/h264_direct.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/h264_direct.c b/libavcodec/h264_direct.c
> > index 4e7202b..20136b0 100644
> > --- a/libavcodec/h264_direct.c
> > +++ b/libavcodec/h264_direct.c
> > @@ -138,7 +138,11 @@ void ff_h264_direct_ref_list_init(const
> H264Context *const h, H264SliceContext *
> >          memcpy(cur->ref_poc[1],   cur->ref_poc[0],
>  sizeof(cur->ref_poc[0]));
> >      }
> >
> > -    cur->mbaff = FRAME_MBAFF(h);
> > +    if (h->current_slice == 0) {
> > +        cur->mbaff = FRAME_MBAFF(h);
> > +    } else if (cur->mbaff != FRAME_MBAFF(h)) {
> > +        av_log(h->avctx, AV_LOG_ERROR, "Frame mixes MBAFF and non-MBAFF
> slices\n");
> > +    }
>
> It should not be possible for mbaff to differ between slices without
> some checks failing
> we check for sps and frame/field to match thogh this code was shuffled
> around with some merges i think it still works
>
> so maybe a assert0 would work for this


Indeed seems to be the case. Check for ff_h264_queue_decode_slice() in
h264_slice.c:2090 for the picture_structure check and just a few lines
above for the SPS check. Changed to av_assert0() in new patch. Thanks for
review.

Ronald
diff mbox

Patch

diff --git a/libavcodec/h264_direct.c b/libavcodec/h264_direct.c
index 4e7202b..20136b0 100644
--- a/libavcodec/h264_direct.c
+++ b/libavcodec/h264_direct.c
@@ -138,7 +138,11 @@  void ff_h264_direct_ref_list_init(const H264Context *const h, H264SliceContext *
         memcpy(cur->ref_poc[1],   cur->ref_poc[0],   sizeof(cur->ref_poc[0]));
     }
 
-    cur->mbaff = FRAME_MBAFF(h);
+    if (h->current_slice == 0) {
+        cur->mbaff = FRAME_MBAFF(h);
+    } else if (cur->mbaff != FRAME_MBAFF(h)) {
+        av_log(h->avctx, AV_LOG_ERROR, "Frame mixes MBAFF and non-MBAFF slices\n");
+    }
 
     sl->col_fieldoff = 0;