[FFmpeg-devel] BUG in mpeg video decoding? ...

[Luca Abeni]

Hi Michael,

On Mon, 2009-06-08 at 01:28 +0200, Michael Niedermayer wrote:
[...]
> > How to reproduce: just generate a "test.mpg" file with output_example,
> > add something like "fprintf(stderr, "Got PTS %Ld\n", picture.pts);"
> > after avcodec_decode_video2() in ffmpeg.c, and run "ffmpeg -i test.mpg
> > t1.mpg".
> > 
> > Is this a bug? Or did I misunderstand something? If it is a bug, I am
> > willing to investigate it... Just point me to the code I should look at
> > (I checked libavcodec/mpeg12.c and libavcodec/mpegvideo.c, and I did not
> > find any reference to "pts"...).
> 
> maybe a missing call to avcodec_get_frame_defaults() / avcidec_alloc_frame()
> somewhere

So, the decoder is responsible for calling avcodec_get_frame_defaults()
or similar?
I ask because I do not know this stuff too much, and I see that only few
decoders call avcodec_get_frame_defaults() or avcodec_alloc_frame()...
I see that avcodec_get_frame_defaults() is called by ffmpeg before
calling avcodec_decode_video2(), but then the MPEG decoder overwrites
the pts value with a 0.

Anyway, I did some investigation and tests, and it seems to me that the
attached patch fixes the issue (with it applied, the mpeg decoder
returns AV_NOPTS_VALUE as a pts) without breaking anything (I hope... I
ran some tests with other decoders, and I do not see regressions).

I am mostly ignorant about these issues, so the patch might be
completely wrong... It just seems to work, but I am not sure if this is
the correct approach. (so, this is probably not meant to be applied, I
send it to get some feedback and see if this is the correct approach or
if I am misunderstanding something - BTW, I had no chance to run "make
test" yet... I'll do it this afternoon).
If it is wrong or must be improved in some way, let me know and I'll
work on it.

> its simply a matter o looking at te get_buffer call and where the frame
> is then finally output, somewhere there or between pts is likely lost

If my understanding of the code is correct, the mpeg{1,2} decoder is
just not setting the pts value. The mpeg4 decoder is setting it, so I
get meaningful pts values. I tried to add a call to
avcodec_get_frame_defaults() before alloc_frame_buffer() in mpegvideo.c
(which calls get_buffer()), and this fixed the mpeg{1,2} case... But
broke mpeg4 decoding, because it forced the mpeg4 decoder to always
return AV_NOPTS_VALUE as a pts (I think the pts is set before calling
alloc_frame_buffer())...
The attached patch is a little bit more complex, but does not present
this problem.


			Thanks,
				Luca
-------------- next part --------------
A non-text attachment was scrubbed...
Name: maybe_fix_decoder_pts-1.diff
Type: text/x-patch
Size: 1178 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090608/01140e02/attachment.bin>