[FFmpeg-cvslog] vsrc_buffer: simplify av_vsrc_buffer_add_frame*() interface

[Michael Niedermayer]

On Sat, May 14, 2011 at 12:25:06AM +0200, Stefano Sabatini wrote:
> On date Saturday 2011-05-14 00:19:05 +0200, Michael Niedermayer wrote:
> > On Sat, May 14, 2011 at 12:03:53AM +0200, Michael Niedermayer wrote:
> > > On Fri, May 13, 2011 at 11:24:47PM +0200, Stefano Sabatini wrote:
> > > > On date Friday 2011-05-13 20:37:13 +0200, Michael Niedermayer wrote:
> > > > > On Fri, May 13, 2011 at 07:07:27PM +0200, Stefano Sabatini wrote:
> > > > > > On date Friday 2011-05-13 16:37:05 +0000, Carl Eugen Hoyos wrote:
> > > > > > > Stefano Sabatini <git <at> videolan.org> writes:
> > > > > > > 
> > > > > > > > ffmpeg | branch: master | Stefano Sabatini | Mon May  2 09:52:11 2011
> > > > > > > > +0200| [64c06615d2b99fb9f4ffd6226d0e5787b44a41e2] | committer: Stefano Sabatini
> > > > > > > > 
> > > > > > > > vsrc_buffer: simplify av_vsrc_buffer_add_frame*() interface
> > > > > > > 
> > > > > > > This broke multi-threaded (H-264) decoding, ticket 197.
> > > > > > 
> > > > > > Check attached, untested but now I have no time and I'm not sure I can
> > > > > > understand which is the correct way to deal with pkt_dts/pts in case
> > > > > > of multithreading.
> > > > > 
> > > > > I think this stuff should be in avcodec_default_get_buffer() (too)
> > > > 
> > > > I believe it is at least partially already done there (with exception
> > > > for the SAR?).
> > > > 
> > > > Michael can you say which is the right way to tackle the problem?
> > > > 
> > > > Should:
> > > > picture->pkt_dts= avpkt->dts;
> > > > picture->pkt_pos= avpkt->pos;
> > > > 
> > > > be set only if threads are disabled?
> > > 
> > > the pos line there is wrong with frame reordering  even without threads.
> > > dts is wrong if threads are used
> > > 
> > > the others are also wrong when frame reoerdering is used.
> > 
> > To explain this a bit more
> > when frames are reordered the frame output and the decoded arent
> > neccessarily the same, the AVCodecContext contains the decoded
> > the AVFrame the output
> > A copy between them is wrong when they differ. (which is rare but that
> > rare case is the whole point behind having these values in AVFrame ..)
> > 
> > I tried to fix it with my last 2 commits.
> 
> Thanks for the explanation and for the fix.

np, its my fault, i missed this when reviewing the patch

also theres another side from which it can be seen

The width/height/format should be set in get_buffer() because its
certainly correct at that point as that code is after all allocating
a frame of these dimensions and format


[..]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-cvslog/attachments/20110514/4ded7837/attachment.asc>