[FFmpeg-devel] [FFmpeg-cvslog] VC1testenc: convert pts values to correct time-base.

[Ronald S. Bultje]

Hi,

On Sat, Jan 29, 2011 at 7:44 AM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de> wrote:
> On Sat, Jan 29, 2011 at 04:13:51AM -0800, Baptiste Coudurier wrote:
>> On 1/29/11 4:08 AM, Reimar D?ffinger wrote:
>> > On Sat, Jan 29, 2011 at 01:05:36PM +0100, Reimar D?ffinger wrote:
>> >> On Sat, Jan 29, 2011 at 03:47:49AM -0800, Baptiste Coudurier wrote:
>> >>> On 1/29/11 3:01 AM, Reimar D?ffinger wrote:
>> >>>> ffmpeg | branch: master | Reimar D?ffinger <Reimar.Doeffinger at gmx.de> | Sat Jan 29 11:56:25 2011 +0100| [76c802e989b61423c1554cf204f96f70b3edb145] | committer: Reimar D?ffinger
>> >>>>
>> >>>> VC1testenc: convert pts values to correct time-base.
>> >>>>
>> >>>> VC1 test container always uses time-base 1 ms, so we must convert
>> >>>> from whatever time-base the application gave us to that, otherwise
>> >>>> the video will play at ridiculous speeds.
>> >>>> It would be possible to signal that a container supports only one
>> >>>> time-base and have code in a layer above do the conversion, but
>> >>>> for a single format this seems over-engineered.
>> >>>>
>> >>>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=76c802e989b61423c1554cf204f96f70b3edb145
>> >>>> ---
>> >>>>
>> >>>> ?libavformat/vc1testenc.c | ? ?5 ++++-
>> >>>> ?1 files changed, 4 insertions(+), 1 deletions(-)
>> >>>>
>> >>>> diff --git a/libavformat/vc1testenc.c b/libavformat/vc1testenc.c
>> >>>> index 507b332..06431da 100644
>> >>>> --- a/libavformat/vc1testenc.c
>> >>>> +++ b/libavformat/vc1testenc.c
>> >>>> @@ -55,11 +55,14 @@ static int vc1test_write_packet(AVFormatContext *s, AVPacket *pkt)
>> >>>> ?{
>> >>>> ? ? ?RCVContext *ctx = s->priv_data;
>> >>>> ? ? ?ByteIOContext *pb = s->pb;
>> >>>> + ? ?uint32_t pts = av_rescale(pkt->pts,
>> >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?1000 * (uint64_t)s->streams[0]->time_base.num,
>> >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?s->streams[0]->time_base.den);
>> >>>
>> >>> This should not be needed if av_set_pts_info is correctly used in
>> >>> write_header.
>> >>
>> >> That means that an application must re-check the time-base after
>> >> av_write_header, that is definitely _not_ documented, or am I
>> >> missing something?
>> >
>> > And I forgot to say that this just pushes the need for conversion
>> > onto the libavformat user, I am not exactly fond of that.
>>
>> This is how it works currently, and since a long time.
>>
>> The user specify the time base using codec->time_base, and the container
>> uses it.
>>
>> Some containers only support one time base (mpeg ps/ts for example
>> 1/90000), so they can't use the one specified.
>>
>> If you have timestamps problems, something is wrong somewhere else, but
>> this hunk should not needed, no muxer do this currently.
>
> I apologize, I read some other muxers and thought this was supposed
> to be done like this, and nobody spoke up when I claimed that during the first
> review.
> I am not completely wrong actually, gxfenc does do this for the audio stream,
> however all others use rescaling only for otehr things (mostly comparing/converting
> per-stream vs. per-file timestamps).
> Does the below change look correct?
>
> diff --git a/libavformat/vc1testenc.c b/libavformat/vc1testenc.c
> index 06431da..3457dbd 100644
> --- a/libavformat/vc1testenc.c
> +++ b/libavformat/vc1testenc.c
> @@ -47,6 +47,7 @@ static int vc1test_write_header(AVFormatContext *s)
> ? ? ? ? put_le32(pb, s->streams[0]->r_frame_rate.den);
> ? ? else
> ? ? ? ? put_le32(pb, 0xFFFFFFFF); //variable framerate
> + ? ?av_set_pts_info(s->streams[0], 32, 1, 1000);
>
> ? ? return 0;
> ?}

Merged this part...

Ronald