[FFmpeg-devel] [PATCH] Pass pts values through non-delay encoders by default.

[Michael Niedermayer]

On Mon, Jan 23, 2012 at 08:54:32AM +0100, Reimar Döffinger wrote:
> On 23 Jan 2012, at 04:46, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Sun, Jan 22, 2012 at 02:39:52PM +0100, Reimar Döffinger wrote:
> >> Avoids having to duplicate the code for trivial, non-reordering
> >> encoders.
> >> Completely and utterly breaks almost all H.264 conformance tests,
> >> sometimes just making the pts start with negative values, sometimes
> >> only giving 2 instead of 20 decoded frames or failing with not
> >> correctly ordered pts.
> >> 
> >> Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> >> ---
> >> libavcodec/utils.c |    6 ++++++
> >> 1 files changed, 6 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> >> index a505fa5..25ad3eb 100644
> >> --- a/libavcodec/utils.c
> >> +++ b/libavcodec/utils.c
> >> @@ -1145,6 +1145,12 @@ int attribute_align_arg avcodec_encode_video(AVCodecContext *avctx, uint8_t *buf
> >>         int ret = avctx->codec->encode(avctx, buf, buf_size, pict);
> >>         avctx->frame_number++;
> >>         emms_c(); //needed to avoid an emms_c() call before every return;
> >> +        if (!(avctx->codec->capabilities & CODEC_CAP_DELAY)) {
> >> +            if (!avctx->coded_frame)
> >> +                avctx->coded_frame = pict;
> > 
> > iiiks
> > coded_frame is supposed to be managed by the encoder, setting it from
> > outside doesnt feel safe. (a failing malloc and its NULL and a free
> > later and theres a double free just as one example)
> > also theres a problem with the lifetime of the objects
> > coded_frame should be valid till the next avcodec* call at least
> > but the pict argument might become invalid sooner than that.
> 
> Sorry, but then the API is just idiotic. You do realise that what you describe means that the rawvideo encoder has to make a copy of the frame to behave correctly?

it just needs a allocated frame, no need to copy

also suggestions to improve it are welcome
but we used coded_frame in the muxers directly a long time ago so the
API definitly was that coded_frame had a longer lifetime than
the AVFrame input


> What is coded_frame used for anyway?

With it the user can know various values the encoder used
like the timestamp, keyframe flag, motion vectors, picture type
macroblock types, ...
motion vectors for example are needed for at least one kind of h263 in
rtp that we dont support yet.


> 
> > its not as generic but i would suggest to move this logic into
> > ffmpeg.c
> > or at least limit the hacking to ->pts
> 
> Then every other application using FFmpeg needs to duplicate the same code.

> I guess saying "null coded_frame means same pts in as out" might work.

that was what i was thinking of


> (hacking only pts doesn't work I think, some codes rather randomly do or do not set coded_frame I had the impression. Rawvideo seems to set it, but with wrong pts, flashsv seems to not set it at all).

pts should be AV_NOPTS_VALUE when unknown never random ...

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

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- 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-devel/attachments/20120123/4b0d9ca1/attachment.asc>