[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