[Ffmpeg-cvslog] r8334 - trunk/libavformat/matroska.c

[Baptiste Coudurier]

Hi

Aurelien Jacobs wrote:
> On Tue, 13 Mar 2007 21:06:47 +0100
> Michael Niedermayer <michaelni at gmx.at> wrote:
> 
>> Hi
>>
>> On Tue, Mar 13, 2007 at 03:35:27PM +0200, Uoti Urpala wrote:
>>> On Tue, 2007-03-13 at 13:53 +0100, Michael Niedermayer wrote:
>>>> the pts->dts code fails if EVERY of the following assumtations is true
>>>>     codec is h.264
>>>>     complex frame reordering is used (like b pyramid)
>>>>     pts are available
>>>>     dts are not available
>>>>     dts are equal to sorted pts
>>> The code also seems to use pkt->duration, which cannot be known
>>> correctly based on pts without demuxing further. Does it always work
>>> right with simpler frame reordering but VFR? Also with the Matroska
>> ive already said that the code works, if you have found a case where
>> it fails (except h.264 stuff), then  please submit a bugreport
>>
>>
>>> sample I tested even the incorrect dts calculation logic was not
>>> triggered at all, but dts was always set equal to pts.
>> well id guess the matroska demuxer or parser forgot to set some things
>> correctly, maybe has_b_frames or the frame type, or maybe theres no
>> parser at all initalized ...
> 
> Now that I read this, this seems so simple !
> I wonder why I didn't thought about setting st->need_parsing myself...
> See quick and dirty attached patch. It disable my custom pts reordering
> code and set st->need_parsing. And this indeed gives a smooth playback
> for soraya.mkv.
> Wait... That's not so simple. With the attached patch, planet.earth.mkv
> don't play at all ! It gives only sound, no video. Probably another
> small stupid mistake, but I'm too tired right now. I will look at
> this tomorrow, and hopefully, we can get ride of my ugly pts reordering
> code.
> 
> Aurel
> 
> 
> ------------------------------------------------------------------------
> 
> Index: libavformat/matroska.c
> ===================================================================
> --- libavformat/matroska.c	(revision 8387)
> +++ libavformat/matroska.c	(working copy)
> @@ -2292,7 +2292,7 @@
>                       codec_id == CODEC_ID_MPEG4      ||
>                       codec_id == CODEC_ID_MSMPEG4V3  ||
>                       codec_id == CODEC_ID_H264) {
> -                track->flags |= MATROSKA_TRACK_REORDER;
> +/*                 track->flags |= MATROSKA_TRACK_REORDER; */
>              }
>  
>              else if (codec_id == CODEC_ID_AAC && !track->codec_priv_size) {
> @@ -2355,6 +2355,13 @@
>              av_set_pts_info(st, 64, matroska->time_scale, 1000*1000*1000); /* 64 bit pts in ns */
>  
>              st->codec->codec_id = codec_id;
> +            if (codec_id == CODEC_ID_MPEG1VIDEO ||
> +                codec_id == CODEC_ID_MPEG2VIDEO ||
> +                codec_id == CODEC_ID_MPEG4      ||
> +                codec_id == CODEC_ID_MSMPEG4V3  ||
> +                codec_id == CODEC_ID_H264) {
> +                st->need_parsing = 2;
> +            }
>  

avi/gxf/mxf demuxer always set need_parsing to 2 (complete frame
parsing), maybe it is worth to do the same with mkv, even if no parser
is available, it should not be harmful.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312