[FFmpeg-devel] [PATCH] probe max read size
Michael Niedermayer
michaelni
Wed Jun 3 04:49:02 CEST 2009
On Mon, Jun 01, 2009 at 09:50:47PM -0700, Baptiste Coudurier wrote:
> Michael Niedermayer wrote:
> > On Mon, Jun 01, 2009 at 03:37:55PM -0700, Baptiste Coudurier wrote:
> >> Michael Niedermayer wrote:
> >>> On Mon, Jun 01, 2009 at 12:36:54PM -0700, Baptiste Coudurier wrote:
> >>>> Michael Niedermayer wrote:
> >>>>> On Sun, May 31, 2009 at 02:53:34AM -0700, Baptiste Coudurier wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> After some tests, it seems more reasonable to stop probing after some
> >>>>>> max size to avoid consuming to much memory.
> >>>>> Maybe we then should replace probe_packets by a size based thing,
> >>>>> having 2 is uglier than one if one is sufficient
> >>>> Sure. Problem is per stream counting is not reasonable since streams
> >>>> might not have many packets present this will cause much buffering.
> >>> but without per stream counting streams starting once the threashold is
> >>> reached should fail to be probed or some streams would only be returned
> >>> at eof ...
> >> I think that, in memory constrained systems, user will want to limit:
> >> - raw_packet_buffer size
> >> - probing data size (which is btw still a problem, pd->buf must be
> >> released at some point if codec is not probed).
> >>
> >> To achieve this, I think both sizes must be globally bounded.
> >>
> >> Maybe by s->probesize or another field, but ideally it must be user
> >> configurable.
> >
> > Either way the value that counts the amount remaining must be in a context
> > not on the stack of av_read_*
> >
> > also if it is in the context it can be set by the user
>
> Yes, definitely.
>
> >>> If we want a packet limit of PL and a byte limit of BL then
> >>>
> >>> init(){
> >>> counter= BL
> >>> }
> >>> ...
> >>> counter -= packet.size + BL/PL;
> >>> if(counter > 0)
> >>>
> >>> could be used, per stream, iam not sure if this is a good idea or if
> >>> we rather should keep 2 counter
> >>>
> >>> Geometrically, this limit check has an area of a triangle that is within the
> >>> limit while the normal 2 variable check uses a rectangle, i feel that a triangle
> >>> is supperior ;)
> >> Sure, is packet limit still wanted if we have global byte limit though ?
> >
> > assuming you have a 1mb size limit and a stream containing gsm
> > it wont ever be probebed successfully because we have no gsm probe
> > it has 13kbit/sec which means that no packet will be returned to the
> > user app for ~10 minutes when then finally all the packets will be
> > returned for these 10min
> >
>
> Patch attached. I believe using probesize is a good opportunity and I
> plan to replace MAX_READ_SIZE by probesize too, changing probesize
> default to 5000000.
>
> I find the FFMAX a bit ugly but I feel it would be safer.
[...]
> @@ -532,8 +534,13 @@ int av_read_packet(AVFormatContext *s, AVPacket *p
> if (pktl) {
> *pkt = pktl->pkt;
> if(s->streams[pkt->stream_index]->codec->codec_id != CODEC_ID_PROBE ||
> - !s->streams[pkt->stream_index]->probe_packets){
> + !s->streams[pkt->stream_index]->probe_packets ||
> + s->raw_packet_buffer_remaining_size <= 0){
> + AVProbeData *pd = &st->probe_data;
> + av_freep(&pd->buf);
> + pd->buf_size = 0;
> s->raw_packet_buffer = pktl->next;
> + s->raw_packet_buffer_remaining_size += pkt->size;
> av_free(pktl);
> return 0;
> }
> @@ -567,6 +574,8 @@ int av_read_packet(AVFormatContext *s, AVPacket *p
> return ret;
>
> add_to_pktbuf(&s->raw_packet_buffer, pkt, &s->raw_packet_buffer_end);
> + s->raw_packet_buffer_remaining_size =
> + FFMAX(0, s->raw_packet_buffer_remaining_size - pkt->size);
>
> if(st->codec->codec_id == CODEC_ID_PROBE){
> AVProbeData *pd = &st->probe_data;
is the combination of FFMAX here and the += pkt->size; above not able to
change raw_packet_buffer_remaining_size to a value that is beyond the
initial limit ?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- 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/20090603/0c6a0c28/attachment.pgp>
More information about the ffmpeg-devel
mailing list