[FFmpeg-devel] [PATCH] H.264 fixinterlacedflag:bringback pic_struct

Michael Niedermayer michaelni
Tue Jun 2 00:41:56 CEST 2009


On Sun, May 31, 2009 at 05:42:37PM +0900, Haruhiko Yamagata wrote:
>> On Fri, May 29, 2009 at 07:18:22PM +0900, Haruhiko Yamagata wrote:
>>>> On Thu, May 28, 2009 at 11:23:17PM +0900, Haruhiko Yamagata wrote:
>>>>>> On Thu, May 28, 2009 at 10:59:09PM +0900, Haruhiko Yamagata wrote:
>>>>>>>> On Thu, May 28, 2009 at 09:24:59PM +0900, Haruhiko Yamagata wrote:
>>>>>>>>> On Tue, Mar 03, 2009, Michael Niedermayer wrote:
>>>>>>>>>> ct_type should always be used if available, otherwise
>>>>>>>>>> FIELD_OR_MBAFF_PICTURE should be.
>>>>>>>>>> that at least is how i understand the spec, if this fails for some
>>>>>>>>>> case iam interrested in the file
>>>>>>>>>
>>>>>>>>> Excuse me for late response, I found a file which fails.
>>>>>>>>> http://x264.nl/h.264.samples/premiere-paff.ts
>>>>>>>>>
>>>>>>>>> Please note that ct_type is optional in the picture timing SEI.
>>>>>>>>>
>>>>>>>>>>            if(get_bits(&s->gb, 1)){                  /* 
>>>>>>>>>> clock_timestamp_flag */
>>>>>>>>>>                unsigned int full_timestamp_flag;
>>>>>>>>>>                h->sei_ct_type |= 1<<get_bits(&s->gb, 2);
>>>>>>>>>
>>>>>>>>> That file does not have ct_type at all, but has correct pic_struct.
>>>>>>>>> pic_struct is basically correct, only a few streams are badly 
>>>>>>>>> encoded.
>>>>>>>>> The attached patch fixes this issue and still honor ct_type if 
>>>>>>>>> available.
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>> Haruhiko Yamagata
>>>>>>>>
>>>>>>>>>  h264.c |   19 +++++++++++++------
>>>>>>>>>  1 file changed, 13 insertions(+), 6 deletions(-)
>>>>>>>>> 832b6de132e2ccb691445f47a3ee3b56d0b1e65d  
>>>>>>>>> check_presence_of_cy_type.patch
>>>>>>>>> Index: libavcodec/h264.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- libavcodec/h264.c (revision 18971)
>>>>>>>>> +++ libavcodec/h264.c (working copy)
>>>>>>>>> @@ -6866,7 +6866,7 @@
>>>>>>>>>          for (i = 0 ; i < num_clock_ts ; i++){
>>>>>>>>>              if(get_bits(&s->gb, 1)){                  /* 
>>>>>>>>> clock_timestamp_flag */
>>>>>>>>>                  unsigned int full_timestamp_flag;
>>>>>>>>> -                h->sei_ct_type |= 1<<get_bits(&s->gb, 2);
>>>>>>>>> +                h->sei_ct_type |= 1<<(get_bits(&s->gb, 2)+1);
>>>>>>>>>                  skip_bits(&s->gb, 1);                 /* 
>>>>>>>>> nuit_field_based_flag */
>>>>>>>>>                  skip_bits(&s->gb, 5);                 /* 
>>>>>>>>> counting_type */
>>>>>>>>
>>>>>>>> why?
>>>>>>>
>>>>>>> So that h->sei_ct_type will be none zero if ct_type exists, even if 
>>>>>>> ct_type is zero.
>>>>>>> In other words, h->sei_ct_type shall be zero if ct_type does not 
>>>>>>> exit.
>>>>>>
>>>>>> so 1<<0 is 0 for you?
>>>>>
>>>>> My mistake.
>>>>> A new patch attached.
>>>> [...]
>>>>> @@ -7819,6 +7823,9 @@
>>>>>                  cur->interlaced_frame = FIELD_OR_MBAFF_PICTURE;
>>>>>              }
>>>>>  +            if (h->sei_ct_type)
>>>>> +                cur->interlaced_frame = (h->sei_ct_type & (1<<1)) != 
>>>>> 0;
>>>>
>>>> this overrides the value set from SEI_PIC_STRUCT_FRAME_TRIPLING:
>>>
>>> Yes, I moved the two lines into if(h->sps.pic_struct_present_flag) block 
>>> and
>>> added check.
>>>
>>> Best regards,
>>> Haruhiko Yamagata
>>
>>>  h264.c |   17 ++++++++++++-----
>>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>> 3d86c0ddddd340d6af20ef493f043900ed02cc43  
>>> check_presence_of_ct_type_3.patch
>>> Index: libavcodec/h264.c
>>> ===================================================================
>>> --- libavcodec/h264.c (revision 18980)
>>> +++ libavcodec/h264.c (working copy)
>>> @@ -7790,14 +7790,18 @@
>>>               /* Signal interlacing information externally. */
>>>              /* Prioritize picture timing SEI information over used 
>>> decoding process if it exists. */
>>> -            if (h->sei_ct_type)
>>> -                cur->interlaced_frame = (h->sei_ct_type & (1<<1)) != 0;
>>> -            else
>>> -                cur->interlaced_frame = FIELD_OR_MBAFF_PICTURE;
>>> -
>>>              if(h->sps.pic_struct_present_flag){
>>>                  switch (h->sei_pic_struct)
>>>                  {
>>> +                case SEI_PIC_STRUCT_FRAME:
>>> +                    cur->interlaced_frame = 0;
>>> +                    break;
>>> +                case SEI_PIC_STRUCT_TOP_FIELD:
>>> +                case SEI_PIC_STRUCT_BOTTOM_FIELD:
>>> +                case SEI_PIC_STRUCT_TOP_BOTTOM:
>>> +                case SEI_PIC_STRUCT_BOTTOM_TOP:
>>> +                    cur->interlaced_frame = 1;
>>> +                    break;
>>>                  case SEI_PIC_STRUCT_TOP_BOTTOM_TOP:
>>>                  case SEI_PIC_STRUCT_BOTTOM_TOP_BOTTOM:
>>>                      // Signal the possibility of telecined film 
>>> externally (pic_struct 5,6)
>>> @@ -7814,6 +7818,9 @@
>>>                      cur->repeat_pict = 4;
>>>                      break;
>>>                  }
>>> +
>>> +                if (h->sei_ct_type && h->sei_pic_struct <= 
>>> SEI_PIC_STRUCT_BOTTOM_TOP)
>>> +                    cur->interlaced_frame = (h->sei_ct_type & (1<<1)) != 
>>> 0;
>>
>> i think that leaves at least one case where interlaced_frame frame is not
>> initialized
>>
>>
>>>              }else{
>>>                  /* Derive interlacing flag from used decoding process. 
>>> */
>>>                  cur->interlaced_frame = FIELD_OR_MBAFF_PICTURE;
>
> In this case, neither pic_struct nor ct_type is available.
> This is the best information that we can have here.
> Both pic_struct and ct_type are under pic_struct_present_flag.

ct_type is under clock_timestamp_flag, pic_struct is not
also ct_type can be set to "unknown" the code should fallback to someting
else in that case

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090602/a9e7f59c/attachment.pgp>



More information about the ffmpeg-devel mailing list