[FFmpeg-devel] [PATCH] H.264 timestamps in h264_parser - complete set

[Ivan Schreter]

Michael Niedermayer wrote:
> On Wed, Feb 18, 2009 at 12:33:57PM +0100, Ivan Schreter wrote:
>   
>>> [...]
>>>   
>>>       
>>>> Index: libavcodec/h264_parser.c
>>>> ===================================================================
>>>> --- libavcodec/h264_parser.c    (revision 17392)
>>>> +++ libavcodec/h264_parser.c    (working copy)
>>>> @@ -171,6 +171,33 @@
>>>>                  /* key frame, since recovery_frame_cnt is set */
>>>>                  s->flags |= PARSER_FLAG_KEY_FRAME;
>>>>              }
>>>> +            pps_id= get_ue_golomb(&h->s.gb);
>>>> +            if(pps_id>=MAX_PPS_COUNT) {
>>>> +                av_log(h->s.avctx, AV_LOG_ERROR, "pps_id out of 
>>>> range\n");
>>>> +                return -1;
>>>> +            }
>>>> +            if(!h->pps_buffers[pps_id]) {
>>>> +                av_log(h->s.avctx, AV_LOG_ERROR, "non-existing PPS 
>>>> referenced\n");
>>>> +                return -1;
>>>> +            }
>>>> +            h->pps= *h->pps_buffers[pps_id];
>>>> +            if(!h->sps_buffers[h->pps.sps_id]) {
>>>> +                av_log(h->s.avctx, AV_LOG_ERROR, "non-existing SPS 
>>>> referenced\n");
>>>> +                return -1;
>>>> +            }
>>>> +            h->sps = *h->sps_buffers[h->pps.sps_id];
>>>> +            h->frame_num = get_bits(&h->s.gb, 
>>>> h->sps.log2_max_frame_num);
>>>> +
>>>> +            if (h->sps.frame_mbs_only_flag) {
>>>> +                /* single picture for a frame */
>>>> +                h->field_pic_flag = 0;
>>>> +            } else {
>>>> +                h->field_pic_flag = get_bits1(&h->s.gb);
>>>> +                if (h->field_pic_flag) {
>>>> +                    h->bottom_field_flag = get_bits1(&h->s.gb);
>>>> +                }
>>>> +            }
>>>> +
>>>>              return 0; /* no need to evaluate the rest */
>>>>          }
>>>>          buf += consumed;
>>>>     
>>>>         
>>> this maybe can be factored with h264.c
>>>   
>>>       
>> Yes. I was thinking about creating additional function like 
>> ff_h264_decode_slice_header_0(), which would contain the first part of 
>> decode_slice_header(), which is relevant for parser. Do you have a better 
>> name suggestion?
>>     
>
> not at the moment but maybe i come up with something later
>
>   
Unfortunately, it's not that simple, since the parsing code there is 
intermixed with other code related to decoding. I'm afraid to break 
things, not badly, but subtly. So I'd prefer not to factor it now, if 
you are OK with it. After the other OK-ed patches are committed, I'll 
post an updated version of parser patches.

Regards,

Ivan