[FFmpeg-devel] [PATCH] make mpeg4 parser set has_b_frames - has_b_frames.patch (0/1)

elupus elupus
Fri Nov 23 00:34:45 CET 2007


On Thu, 22 Nov 2007 02:41:29 +0100, Michael Niedermayer
<michaelni at gmx.at> wrote:

>On Thu, Nov 22, 2007 at 02:09:11AM +0100, elupus wrote:
[...]
>
>of course, compute_pkt_fields() is the fast "just fill in what we can with no
>knowledge of the future" code
>genpts is the "find the pts even if we have to read ahead the whole file" code
>
>
[...]
>> But I assume you won't be bothered to look so the above was probably a waste 
>> of typing.
>
>if has_b_frames==0 there are no b frames, so dts==pts
>you cant expect code to provide you with correct output if you feed
>it with incorrect input
>
>[...]

Well, true, but as I see it has_b_frames == 0 is the default value,
and and not all parsers can set a correct value for it, as in the case
was with the mpeg4 parser.

if demuxer can't provide you with a pts, only dts. assuming in that
case that pts==dts is rather bad (if no parse that is able to set
has_b_frame correctly is available). If pts was left as nopts, player
could use the still correct dts for playback. while in it's current
way it risks using an incorrect pts for playback resulting in poor
playback. the problem here being that player have no idea if dts or
pts is the best choice to sync on since both gets set.

if genpts is set, then fine make the assumption that parser gets it
right cause user have asked us ffmpeg to figure out pts at any cost,
and then we have to assume parser will do a correct job.

I suppose this type of change might require a version bump since it
changes ffmpeg's default from always filling pts, to only doing so if
it's likely correct.


anyway, i've resent the patch. let's hope this news client get's it
right.

Joakim





More information about the ffmpeg-devel mailing list