[FFmpeg-devel] [PATCH] Vorbis-in-Ogg: Do not set timebase to invalid values

[Reimar Döffinger]

On Sat, Jan 29, 2011 at 12:41:22PM +0100, Reimar D?ffinger wrote:
> On Sat, Jan 29, 2011 at 03:32:17AM -0800, Baptiste Coudurier wrote:
> > On 1/29/11 3:06 AM, Ronald S. Bultje wrote:
> > >> I was going to say that there is no place to do this that does not include
> > >> a lot of issues, but there actually is one:
> > >> av_set_pts_info
> > >> With the little issue that this code incorrectly bypasses that function.
> > >> A lot of code does that, so a first patch is to make everyone actually
> > >> use that.
> > >> I probably missed an few.
> > > 
> > > I really like this patch. Might want a few people to check and make
> > > sure we didn't miss any, but this looks very right.
> > 
> > Yes, that's the way to do it, but av_set_pts_info has a problem:
> > It's using unsigned as parameters while AVRational is two int, and I'm
> > not sure how av_reduce exactly behaves.
> 
> Just to clarify: The patch is completely useless for fixing the issue,
> av_set_pts_info does not sensible checking, and the only checking it
> does sets the time_base to values that _will_ trigger that assert.
> So there's a good chance it makes things even worse.
> But it is the only sensible way I can see to get a check that
> works without extra code in every demuxer.
> I does need someone investing the time though, and I am not
> sure I'll feel like going beyond this.

Sorry, one more thing: the fundamental issue of av_set_pts_info
that Michael has complained about several times in other code
is that it _first_ assigns the values and _then_ does the
validation (next come the facts that the validation is
insufficient and then that due to already having overwritten
the previous values is has not much choice except to set them
to inappropriate, and in this case crashing, values).