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

[Michael Niedermayer]

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.


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- 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/20100702/0d33be38/attachment.pgp>