[FFmpeg-devel] [RFC][PATCH] return packets for known codecs earlier when codec probing is done

[Michael Niedermayer]

On Sun, May 24, 2009 at 03:03:33PM -0700, Baptiste Coudurier wrote:
> On 5/23/2009 5:58 PM, Baptiste Coudurier wrote:
> > On 5/23/2009 5:49 PM, Michael Niedermayer wrote:
> >> On Sat, May 23, 2009 at 03:56:11PM -0700, Baptiste Coudurier wrote:
> >>> Hi,
> >>>
> >>> $subject.
> >>>
> >>> I'd like comments on this. I don't really get why we wouldn't return
> >>> packets for known codecs. Why do we buffer them ?
> >> if we have packets in the buffer we must return them first, before the
> >> current packet even if the codec is known, adding the current packet
> >> to the buffer is needed as we return an earlier packet first. And cant
> >> return 2 at once.
> >> But maybe you meant something else?
> > 
> > Ok.
> > 
> >>> Problem is that packets for known codecs get buffered but never returned
> >>> until codec is sucessfully probed for the unkwnown stream, which might
> >>> not even happen.
> >> That sounds like a seperat issue, 
> >> There could be a size limit at which we turn PROBE in NONE and return
> >> stuff ...
> > 
> > Yes, that's what's needed.
> 
> What about this ?
> 
> flush_packet_queue needs to flush raw_packet_buffer too.
> 
> Do you have samples that need probing I could test against ?

i certainly do but i dont think i would find them, some MPEG files ...


> 
> -- 
> Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
> FFmpeg maintainer                                  http://www.ffmpeg.org

>  utils.c |   25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 5b9267140a3e8288686f1532a7761bc5d8fb9a5f  stop_codec_probing.patch
> Index: libavformat/utils.c
> ===================================================================
> --- libavformat/utils.c	(revision 18912)
> +++ libavformat/utils.c	(working copy)
> @@ -520,7 +520,7 @@ static AVPacket *add_to_pktbuf(AVPacketList **pack
>  
>  int av_read_packet(AVFormatContext *s, AVPacket *pkt)
>  {
> -    int ret;
> +    int ret, i, read_size = 0;
>      AVStream *st;
>  
>      for(;;){

i think the read_size variable will in some form have to be put in AVStream


> @@ -537,10 +537,18 @@ int av_read_packet(AVFormatContext *s, AVPacket *p
>  
>          av_init_packet(pkt);
>          ret= s->iformat->read_packet(s, pkt);
> -        if (ret < 0)
> -            return ret;
> +        if (ret < 0) {
> +            if (!pktl)
> +                return ret;
> +            for (i = 0; i < s->nb_streams; i++)
> +                if (s->streams[i]->codec->codec_id == CODEC_ID_PROBE)
> +                    s->streams[i]->codec->codec_id = CODEC_ID_NONE;
> +            continue;
> +        }

i dont like this


>          st= s->streams[pkt->stream_index];
>  
> +        read_size += pkt->size;
> +

I think read_size++ is more robust in light of streams of widely
varriing bitrate 10mbit video vs. a few kbit speech codec


>          switch(st->codec->codec_type){
>          case CODEC_TYPE_VIDEO:
>              if(s->video_codec_id)   st->codec->codec_id= s->video_codec_id;
> @@ -561,6 +569,9 @@ int av_read_packet(AVFormatContext *s, AVPacket *p
>          if(st->codec->codec_id == CODEC_ID_PROBE){
>              AVProbeData *pd = &st->probe_data;
>  
> +            if(read_size >= s->probesize)
> +                st->codec->codec_id = CODEC_ID_NONE;
> +
>              pd->buf = av_realloc(pd->buf, pd->buf_size+pkt->size+AVPROBE_PADDING_SIZE);
>              memcpy(pd->buf+pd->buf_size, pkt->data, pkt->size);
>              pd->buf_size += pkt->size;
[...]


> @@ -1081,6 +1092,14 @@ static void flush_packet_queue(AVFormatContext *s)
>          av_free_packet(&pktl->pkt);
>          av_free(pktl);
>      }
> +    for(;;) {
> +        pktl = s->raw_packet_buffer;
> +        if (!pktl)
> +            break;
> +        s->raw_packet_buffer = pktl->next;
> +        av_free_packet(&pktl->pkt);
> +        av_free(pktl);
> +    }

while(s->raw_packet_buffer){
    pktl = s->raw_packet_buffer;
    s->raw_packet_buffer = pktl->next;
    av_free_packet(&pktl->pkt);
    av_free(pktl);
}


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- 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/20090525/8d0948a6/attachment.pgp>