[Ffmpeg-devel] [RFC] av_seek_frame behaviour

Reimar Döffinger Reimar.Doeffinger
Sun Apr 22 20:04:58 CEST 2007


Hello,
On Sun, Apr 22, 2007 at 06:45:12PM +0200, Baptiste Coudurier wrote:
> Reimar D?ffinger a ?crit :
> > On Sun, Apr 22, 2007 at 04:13:42PM +0200, Baptiste Coudurier wrote:
> >> Reimar D?ffinger a ?crit :
> >>> On Sun, Apr 22, 2007 at 02:50:52AM +0200, Baptiste Coudurier wrote:
> >>>>  I think about always returning after demuxer seek function, see patch.
> >>>>  It might be wrong, and missed some demuxers behaviour.
> >>> This will most likely break seeking for gxf files without index.
> >> Ouch, therefore problem is there for 10gb without index GXF files as 
> >> well. Btw, it would be very much more efficient to seek on MEDIA packet 
> >> for GXF, when no index is found.
> > 
> > That is what av_seek_frame_binary does for gxf. And your patch removes
> > that av_seek_frame_binary fallback...
> 
> Humm it seems av_seek_frame_binary does use index when it is present, 
> why not always using av_seek_frame_binary for GXF ?

Maybe it just didn't when I wrote the code. Or I just missed that fact.
If it works, sure...

> >>> If anything, introduce two different return values, one indicating that
> >>> the frame definitely is not there and another one to indicate that we
> >>> should try the other seek methods.
> >>> But that still will not fix your problem. From the description I'd say
> >>> what you want is a FAST_SEEK_ONLY flag or similar that would only allow
> >>> seek methods with O(log n) max.
> 
> Basically, no. I'd like the seek to just fail and not trying anything 
> else, since there is absolutely no packet with that timestamp if MXF 
> seek function fails.

Even with other formats, that is assuming the file is not broken...
In the case of MXF, this seems to be a simple bit-rate based seek and in
addition there is also
> if (!s->bit_rate)
>   return -1;

So either it can fail even in cases where the time exists or that should
be an assert ;-)
There also is a "/* rudimentary binary seek */" comment above
mxf_read_seek, but I can't see any binary search being done? Or what
should that comment tell me?

[...]
> >>> (which is another things that IMO is
> >>> wrong with that patch, av_seek_frame_binary is O(log n), so except for
> >>> streamed content there is not much reason to try that as well.
> >> That has nothing to do with my patch, current code will always try 
> >> av_seek_frame_binary if demuxer has read_timestamp and no seek function 
> >> or if its seek function returned < 0.
> > 
> > ?? Why do my suggestions to fix the problem in a "proper" way (i.e. without
> > removing the feature of falling back to possibly more reliable seek methods
> > as your patch does) have nothing to do with your patch?
> 
> What's you suggestion ? "av_seek_frame_binary is O(log n), so except for 
> stream context there is not much reason to try that as well ?"

Oh, I meant "not much reason to no try", though that might be
disputable, too. The suggestions I was referring to are the stuff above
what you quoted. I didn't expect your reply was only referring to my
side not in ()...

> I get that to mean that there is no point using av_seek_frame_binary 
> except for stream context, and that has nothing to do with that patch.

I was only pondering if a SEEK_FLAG_NOBINARY would even make sense...

[...]
> >> What about calling av_seek_frame_generic only for formats having 
> >> AVFMT_GENERIC_INDEX ?
> > 
> > Well, IMO any such fixed problem does not solve what I feel is the real
> > problem: Some people will want fast seeks, others need them to be as
> > precise as possible.
> 
> Real problem is to fail quickly when it has to fail. Trying other 
> methods is a plus. So maybe returning AVSEEK_FAIL, AVSEEK_TRY_BINARY, 
> AVSEEK_TRY_GENERIC, or AVSEEK_FLAG_TRY_ALL user can pass.
> 
> A failing demuxer seek function should not mean that other methods 
> should be tried.

Depends on the meaning of "fail". It could "fail" because that position
absolutely certainly isn't there. Or it could "fail" because it tries to
lazily read the index and finds out it can't e.g. because it's
corrupted.
Of course the fallback to other methods could be done in each demuxer,
but using return values to indicate how the seek failed seems more
robust and simpler to me in the long term.

> av_seek_frame_generic when no index is present does read all packets 
> since the beginning that is clearly too expansive and should not be done 
> unless explicitely asked.

I agree it should be possible to disable, but "clearly too expansive" (I
assume you meant expensive) maybe be true in your case, but for
lowres MPEG4-compressed movies stored on HD of around 800 MB (which I would
consider more the normal case) it seems still acceptable to me.

[...]
> > And what about av_seek_frame_binary?  till always calling it whenever
> > the "main" seek function is fails?
> 
> IMO if demuxer knows that av_seek_frame_binary works, it should call it
> itself from its seek function.

Or it could indicate that fact via the return value, which would allow
to avoid code duplication and maintain backwards-compatibility, or does
someone volunteer to check all demuxers if they make assume that
fallback like gxf does? ;-)

Greetings,
Reimar D?ffinger




More information about the ffmpeg-devel mailing list