[FFmpeg-devel] h264: Correctly initialize interlaced_frame if tff is set

Submitted by Vittorio Giovara on Feb. 10, 2017, 10:21 p.m.

Details

Message ID 20170210222100.15027-1-vittorio.giovara@gmail.com
State New
Headers show

Commit Message

Vittorio Giovara Feb. 10, 2017, 10:21 p.m.
In particular cases, it is possible to initialize top_field_first
but not interlaced_frame. Make sure to correctly tag a frame
as interlaced when this happens.

Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
---
Please CC.
Vittorio

 libavcodec/h264_slice.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Michael Niedermayer Feb. 11, 2017, 1:56 a.m.
On Fri, Feb 10, 2017 at 05:21:00PM -0500, Vittorio Giovara wrote:
> In particular cases, it is possible to initialize top_field_first
> but not interlaced_frame. Make sure to correctly tag a frame
> as interlaced when this happens.
> 
> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> ---
> Please CC.
> Vittorio
> 
>  libavcodec/h264_slice.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index 91a3b25..eeb5202 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -1174,20 +1174,23 @@ static int h264_export_frame_props(H264Context *h)
>  
>      if (cur->field_poc[0] != cur->field_poc[1]) {
>          /* Derive top_field_first from field pocs. */
> -        cur->f->top_field_first = cur->field_poc[0] < cur->field_poc[1];
> +        cur->f->interlaced_frame =
> +        cur->f->top_field_first  = cur->field_poc[0] < cur->field_poc[1];

this looks like it would set interlaced_frame = 0 if
cur->field_poc[0] > cur->field_poc[1];

also, do you have a sample that is affected by this ?

[...]
Michael Niedermayer Feb. 14, 2017, 11:21 p.m.
On Sat, Feb 11, 2017 at 02:56:32AM +0100, Michael Niedermayer wrote:
> On Fri, Feb 10, 2017 at 05:21:00PM -0500, Vittorio Giovara wrote:
> > In particular cases, it is possible to initialize top_field_first
> > but not interlaced_frame. Make sure to correctly tag a frame
> > as interlaced when this happens.
> > 
> > Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> > ---
> > Please CC.
> > Vittorio
> > 
> >  libavcodec/h264_slice.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > index 91a3b25..eeb5202 100644
> > --- a/libavcodec/h264_slice.c
> > +++ b/libavcodec/h264_slice.c
> > @@ -1174,20 +1174,23 @@ static int h264_export_frame_props(H264Context *h)
> >  
> >      if (cur->field_poc[0] != cur->field_poc[1]) {
> >          /* Derive top_field_first from field pocs. */
> > -        cur->f->top_field_first = cur->field_poc[0] < cur->field_poc[1];
> > +        cur->f->interlaced_frame =
> > +        cur->f->top_field_first  = cur->field_poc[0] < cur->field_poc[1];
> 
> this looks like it would set interlaced_frame = 0 if
> cur->field_poc[0] > cur->field_poc[1];
> 
> also, do you have a sample that is affected by this ?

thx for the sample, ive pushed a different fix
does that fix this completely or is some issue remaining ?

thx

[...]
Vittorio Giovara Feb. 15, 2017, 2:26 p.m.
On Tue, Feb 14, 2017 at 6:21 PM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Sat, Feb 11, 2017 at 02:56:32AM +0100, Michael Niedermayer wrote:
>> On Fri, Feb 10, 2017 at 05:21:00PM -0500, Vittorio Giovara wrote:
>> > In particular cases, it is possible to initialize top_field_first
>> > but not interlaced_frame. Make sure to correctly tag a frame
>> > as interlaced when this happens.
>> >
>> > Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
>> > ---
>> > Please CC.
>> > Vittorio
>> >
>> >  libavcodec/h264_slice.c | 13 ++++++++-----
>> >  1 file changed, 8 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
>> > index 91a3b25..eeb5202 100644
>> > --- a/libavcodec/h264_slice.c
>> > +++ b/libavcodec/h264_slice.c
>> > @@ -1174,20 +1174,23 @@ static int h264_export_frame_props(H264Context *h)
>> >
>> >      if (cur->field_poc[0] != cur->field_poc[1]) {
>> >          /* Derive top_field_first from field pocs. */
>> > -        cur->f->top_field_first = cur->field_poc[0] < cur->field_poc[1];
>> > +        cur->f->interlaced_frame =
>> > +        cur->f->top_field_first  = cur->field_poc[0] < cur->field_poc[1];
>>
>> this looks like it would set interlaced_frame = 0 if
>> cur->field_poc[0] > cur->field_poc[1];
>>
>> also, do you have a sample that is affected by this ?
>
> thx for the sample, ive pushed a different fix
> does that fix this completely or is some issue remaining ?

Looks good, thanks for the fix.

Patch hide | download patch | download mbox

diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 91a3b25..eeb5202 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1174,20 +1174,23 @@  static int h264_export_frame_props(H264Context *h)
 
     if (cur->field_poc[0] != cur->field_poc[1]) {
         /* Derive top_field_first from field pocs. */
-        cur->f->top_field_first = cur->field_poc[0] < cur->field_poc[1];
+        cur->f->interlaced_frame =
+        cur->f->top_field_first  = cur->field_poc[0] < cur->field_poc[1];
     } else {
         if (sps->pic_struct_present_flag) {
             /* Use picture timing SEI information. Even if it is a
              * information of a past frame, better than nothing. */
             if (h->sei.picture_timing.pic_struct == SEI_PIC_STRUCT_TOP_BOTTOM ||
-                h->sei.picture_timing.pic_struct == SEI_PIC_STRUCT_TOP_BOTTOM_TOP)
-                cur->f->top_field_first = 1;
-            else
+                h->sei.picture_timing.pic_struct == SEI_PIC_STRUCT_TOP_BOTTOM_TOP) {
+                cur->f->interlaced_frame =
+                cur->f->top_field_first  = 1;
+            } else
                 cur->f->top_field_first = 0;
         } else if (cur->f->interlaced_frame) {
             /* Default to top field first when pic_struct_present_flag
              * is not set but interlaced frame detected */
-            cur->f->top_field_first = 1;
+            cur->f->interlaced_frame =
+            cur->f->top_field_first  = 1;
         } else {
             /* Most likely progressive */
             cur->f->top_field_first = 0;