[FFmpeg-devel] [PATCH]H.264fixinterlacedflag:bringback pic_struct
Haruhiko Yamagata
h.yamagata
Wed Jun 3 11:58:02 CEST 2009
>On Tue, Jun 02, 2009 at 08:59:10PM +0900, Haruhiko Yamagata wrote:
>>> 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
>>
>> The spec says
>>> If CpbDpbDelaysPresentFlag is equal to 1 or pic_struct_present_flag is
>>> equal to 1,
>>> one picture timing SEI message shall be present in every access unit of
>>> the coded
>>> video sequence.
>>
>> So if pic_struct_present_flag is equal to 1, decode_picture_timing shall be
>> called
>> and h->sei_ct_type shall be flagged unknown (0) or set correct value.
>
>0 is not the only value that means unknown
>
>
>>
>> In my patch,
>>
>>> + if (h->sei_ct_type && h->sei_pic_struct <=
>>
>> Presence of ct_type is checked here. This is only referenced if
>> pic_struct_present_flag is equal to 1.
>
>and if sei_ct_type==0 interlaced_frame is left at whichever value it was
>previously it was guessed based on field/frame coding
Thank you for your patience. I finally understand.
A new patch attached. Soft telecine is flagged progressive.
Best regards,
Haruhiko Yamagata
-------------- next part --------------
A non-text attachment was scrubbed...
Name: check_presence_of_ct_type_4.patch
Type: application/octet-stream
Size: 2166 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090603/b227f072/attachment.obj>
More information about the ffmpeg-devel
mailing list