Re: [libav-devel] [PATCH 06/15] h264: handle complementary field pairs in await_references()
Hi,
On Wed, Dec 12, 2012 at 4:29 PM, Janne Grunau wrote:
> On 2012-12-12 16:16:21 -0800, Ronald S. Bultje wrote:
>> Hi,
>>
>> On Wed, Dec 12, 2012 at 3:22 PM, Janne Grunau 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
>> >> 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.
>>
>> 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.
>
> I need to check
>
>> What does the hang look like?
>
> ref_field = ref_pic->f.reference;
>
> and it waits on ref_field in the next else that breaks when reference is
> PICT_FRAME
that's BOTTOM_FIELD | TOP_FIELD.
Ronald
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 06/15] h264: handle complementary field pairs in await_references()
On 2012-12-12 16:16:21 -0800, Ronald S. Bultje wrote:
> Hi,
>
> On Wed, Dec 12, 2012 at 3:22 PM, Janne Grunau 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
> >> 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.
>
> 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.
I need to check
> What does the hang look like?
ref_field = ref_pic->f.reference;
and it waits on ref_field in the next else that breaks when reference is
PICT_FRAME
Janne
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 06/15] h264: handle complementary field pairs in await_references()
Hi,
On Wed, Dec 12, 2012 at 3:22 PM, Janne Grunau 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
>> 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.
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.
What does the hang look like?
Ronald
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 06/15] h264: handle complementary field pairs in await_references()
On 2012-12-12 14:39:34 -0800, Ronald S. Bultje wrote:
> Hi,
>
> On Wed, Dec 12, 2012 at 12:30 PM, Janne Grunau 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.
Janne
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 06/15] h264: handle complementary field pairs in await_references()
Hi,
On Wed, Dec 12, 2012 at 12:30 PM, Janne Grunau 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?
Ronald
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
