On 2012-12-12 16:16:21 -0800, Ronald S. Bultje wrote:
> 
> On Wed, Dec 12, 2012 at 3:22 PM, Janne Grunau <janne-li...@jannau.net> wrote:
> > On 2012-12-12 14:39:34 -0800, Ronald S. Bultje wrote:
> >> Hi,
> >>
> >> On Wed, Dec 12, 2012 at 12:30 PM, Janne Grunau <janne-li...@jannau.net> 
> >> wrote:
> >> > Fixes hang in HPCAMAPALQ_BRCM_B.264_s14038 while waiting on invalid
> >> > field 2.
> >> > ---
> >> >  libavcodec/h264.c | 3 ++-
> >> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> >> > index 546b046..d5a54e2 100644
> >> > --- a/libavcodec/h264.c
> >> > +++ b/libavcodec/h264.c
> >> > @@ -433,7 +433,8 @@ static void await_references(H264Context *h)
> >> >                      ff_thread_await_progress(&ref_pic->f,
> >> >                                               FFMIN((row >> 1), 
> >> > pic_height - 1),
> >> >                                               0);
> >> > -                } else if (FIELD_PICTURE && !ref_field_picture) { // 
> >> > field referencing one field of a frame
> >> > +                } else if (FIELD_PICTURE &&
> >> > +                           (!ref_field_picture || ref_field > 1)) { // 
> >> > field referencing one field of a frame or complementary field pair
> >>
> >> I don't understand this one. If we're referencing two fields,
> >> shouldn't ref_field_picture automatically be true?
> >
> > it isn't and the code that marks both fields of a complementory field
> > pair as available doesn't touch field_picture.
> 
> Right, but isn't this wrong then? I mean, the reference is
> field-based, so we're going to reference a field. The part where both
> fields are available doesn't negate the fact that we're only
> referencing one.

Yes the patch is wrong, I wasn't paying enough attention.
A complementary ref field pair can't handled in the same way as a frame.
We reported progress on the individual fields. I don't know how to
determine which field it references so the patch waits on both. eld
 
> It seems to me that a change like this could potentially cause use of
> reference data before the second field of a reference is fully
> decoded.

Yes, see above.

Janne

---8<---
Fixes hang in HPCAMAPALQ_BRCM_B.264_s14038 while waiting on invalid
field 2.
---
 libavcodec/h264.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libavcodec/h264.c b/libavcodec/h264.c
index f01ff67..be2a98b 100644
--- a/libavcodec/h264.c
+++ b/libavcodec/h264.c
@@ -438,6 +438,11 @@ static void await_references(H264Context *h)
                                              FFMIN(row * 2 + ref_field,
                                                    pic_height - 1),
                                              0);
+                } else if (FIELD_PICTURE && ref_field_picture && ref_field == 
2) { // field referencing one field of a complementary field pair
+                    ff_thread_await_progress(&ref_pic->f,
+                                             FFMIN(row, pic_height - 1), 1);
+                    ff_thread_await_progress(&ref_pic->f,
+                                             FFMIN(row, pic_height - 1), 0);
                 } else if (FIELD_PICTURE) {
                     ff_thread_await_progress(&ref_pic->f,
                                              FFMIN(row, pic_height - 1),
-- 
1.7.12.4

_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to