[FFmpeg-devel] [PATCH] H.264 fix interlaced flag: bringback pic_struct

[Haruhiko Yamagata]

>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.
It is necessary for the following code to work.

>            if (h->sei_ct_type)
>                cur->interlaced_frame = (h->sei_ct_type & (1<<2)) != 0;

I admit this is not very beautiful. Do I have to add a flag in the context?
Or should I rename sei_ct_type to sei_ct_type_plus1?

Best regards,
Haruhiko Yamagata