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

[Michael Niedermayer]

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:

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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/20090528/4d9c9737/attachment.pgp>