Hi,
On Fri, Dec 14, 2012 at 7:02 AM, Janne Grunau <janne-li...@jannau.net> wrote:
> 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) {
I still don't get it, how does a field reference need to wait for two
fields? The idea of a field reference is that we only need the data of
1 field.
Can I get the sample? I'll try to see what's going on this weekend
perhaps... All this looks weird.
Ronald
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
