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

[Michael Niedermayer]

On Thu, Jul 01, 2010 at 06:18:38PM -0700, Baptiste Coudurier wrote:
> 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.

it takes additional time and allocates additional memory.
with many streams (maybe 20 audio streams in different languages)
that could on slow hardware cause a noticeable delay < 1 sec though.
But if its needed for fixing some issue? then iam not opposed

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- 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/36b0d162/attachment.pgp>