[FFmpeg-devel] [PATCH] decode at least 4 H.264 frames in av_find_stream_info

[Baptiste Coudurier]

On 07/01/2010 05:03 PM, Michael Niedermayer wrote:
> On Mon, Jun 28, 2010 at 07:34:58PM -0700, Baptiste Coudurier wrote:
>> On 06/27/2010 07:03 PM, Michael Niedermayer wrote:
>>> On Sun, Jun 27, 2010 at 05:44:46PM -0700, Baptiste Coudurier wrote:
>>>> On 6/27/10 3:34 PM, Michael Niedermayer wrote:
>>>>> On Sun, Jun 27, 2010 at 02:18:18PM -0700, Baptiste Coudurier wrote:
>>>>>> On 6/27/10 7:09 AM, Michael Niedermayer wrote:
>>>>>>> On Sun, Jun 27, 2010 at 12:39:13AM -0700, Baptiste Coudurier wrote:
>>>>>>>> On 6/26/10 7:43 PM, Michael Niedermayer wrote:
>>>>>>>>> On Sun, Jun 27, 2010 at 04:31:41AM +0200, Michael Niedermayer wrote:
>>>>>>>>>> On Thu, Jun 24, 2010 at 05:44:20PM -0700, Baptiste Coudurier wrote:
>>>>>>>>>>> Hi guys,
>>>>>>>>>>>
>>>>>>>>>>> $subject.
>>>>>>>>>>>
>>>>>>>>>>> This will permit the decoder to compute has_b_frames correctly
>>>>>>>>>>> when
>>>>>>>>>>> b-pyramid is present. 4 is typically the minimum needed, though
>>>>>>>>>>> maybe
>>>>>>>>>>> we
>>>>>>>>>>> should decode more based on has_b_frames. Any opinion ?
>>>>>>>>>>
>>>>>>>>>> Finally found an old patch by myself doing this
>>>>>>>>>> I think its a bit more readable
>>>>>>>>>> if it works feel free to apply mine (assuming that thing still
>>>>>>>>>> applies
>>>>>>>>>> and
>>>>>>>>>> works)
>>>>>>>>>
>>>>>>>>> ehm, the patch:
>>>>>>>>>
>>>>>>>>> Index: libavformat/utils.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- libavformat/utils.c	(revision 18646)
>>>>>>>>> +++ libavformat/utils.c	(working copy)
>>>>>>>>> @@ -1820,6 +1820,12 @@
>>>>>>>>>       #endif
>>>>>>>>>       }
>>>>>>>>>
>>>>>>>>> +/*we need to find has_b_frames approximatly, the parser is not yet
>>>>>>>>> able
>>>>>>>>> to do it*/
>>>>>>>>> +static int has_decode_delay_been_guessed(AVCodecContext *enc)
>>>>>>>>> +{
>>>>>>>>> +    return enc->codec_id != CODEC_ID_H264 || enc->frame_number>= 4
>>>>>>>>> +
>>>>>>>>> enc->has_b_frames;
>>>>>>>>> +}
>>>>>>>>
>>>>>>>> I'm not sure about enc->frame_number, now that
>>>>>>>> st->codec_info_nb_frames
>>>>>>>> is
>>>>>>>> used in av_find_stream_info, it seems the best field to use.
>>>>>>>
>>>>>>> hmm, i dont think theres a difference currently between using these 2
>>>>>>> though they are semantically a bit different. So use what you prefer
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Your patch is not enough I think, you need to always call
>>>>>>>> try_decode_frame
>>>>>>>> even if has_codec_parameters is true.
>>>>>>>
>>>>>>> calling it for the first 4 frames with h264 is ok
>>>>>>> calling it unconditionally always would slow av_find_stream_info()
>>>>>>> down
>>>>>>> quite
>>>>>>> a bit i think
>>>>>>>
>>>>>>
>>>>>> This is code in svn:
>>>>>>
>>>>>>            if (!has_codec_parameters(st->codec))
>>>>>>                try_decode_frame(st, pkt);
>>>>>>
>>>>>> I meant the check has to be removed, otherwise only one frame will be
>>>>>> decoded.
>>>>>
>>>>> what about:
>>>>>
>>>>> if (!has_codec_parameters(st->codec) ||
>>>>> !has_decode_delay_been_guessed())
>>>>> ?
>>>>
>>>> Why duplicating the check ? This check is also in try_decode_frame, so
>>>> either one can be removed.
>>>
>>> i wish it could but they arent useless
>>> its "if we dont have parameter yet open codec"
>>> and "if we still dont have parameers yet decode frame"
>>>
>>> droping the first will open codecs for all streams, droping the second
>>> will decode frames even if its unneeded after opening the codec
>>
>> What about that ?
>
> I think with this a codec that has its parameters filled in by its
> avcodec_open() will have its first frame decoded unneccesarily.

Yes, do you see anything wrong with that ? Always decoding the first 
frame might be a benefit, I actually have a patch in my tree for that.

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org