[FFmpeg-devel] [PATCH] flvenc: Fix sequence header update timestamps

Steven Liu lq at chinaffmpeg.org
Fri May 4 06:03:20 EEST 2018



> On 4 May 2018, at 02:00, Jan Ekström <jeebjp at gmail.com> wrote:
> 
> On Thu, May 3, 2018 at 8:58 PM, Jan Ekström <jeebjp at gmail.com> wrote:
>> On Thu, May 3, 2018 at 7:50 PM, Alex Converse <alex.converse at gmail.com> wrote:
>>> From: Alex Converse <alexconv at twitch.tv>
>>> 
>>> ---
>>> libavformat/flvenc.c | 10 +++++-----
>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c
>>> index e8af48cb64..827d798a61 100644
>>> --- a/libavformat/flvenc.c
>>> +++ b/libavformat/flvenc.c
>>> @@ -480,7 +480,7 @@ static int unsupported_codec(AVFormatContext *s,
>>>     return AVERROR(ENOSYS);
>>> }
>>> 
>>> -static void flv_write_codec_header(AVFormatContext* s, AVCodecParameters* par) {
>>> +static void flv_write_codec_header(AVFormatContext* s, AVCodecParameters* par, unsigned int ts) {
>> 
>> Hi,
>> 
>> While the timestamp field is 8+24=32 bits, the DTS value is int64_ts.
>> Thus, while I'm really bad at doing wrap-around logic, wouldn't it be
>> something a la ´uint32_t wrapped_timestamp = (uint32_t)(ts %
>> UINT32_MAX)`?
>> 
>> Additionally, I did briefly go through E.4.1 (FLV Tag) in the spec and
>> it doesn't really seem to define whether this value is PTS or DTS...
>> Is this defined anywhere proper? Given FLV's age I wouldn't be
>> surprised that the answer would be "effectively DTS", of course. But
>> if you've seen what  Adobe's implementation writes with B-frames it'd
>> be interesting to see.
>> 
>>>     int64_t data_size;
>>>     AVIOContext *pb = s->pb;
>>>     FLVContext *flv = s->priv_data;
>>> @@ -492,8 +492,8 @@ static void flv_write_codec_header(AVFormatContext* s, AVCodecParameters* par) {
>>>                 par->codec_type == AVMEDIA_TYPE_VIDEO ?
>>>                         FLV_TAG_TYPE_VIDEO : FLV_TAG_TYPE_AUDIO);
>>>         avio_wb24(pb, 0); // size patched later
>>> -        avio_wb24(pb, 0); // ts
>>> -        avio_w8(pb, 0);   // ts ext
>>> +        avio_wb24(pb, ts & 0xFFFFFF);
>>> +        avio_w8(pb, (ts >> 24) & 0x7F);
>> 
>> Looking at the spec this is bottom 24 bits of the timestamp and top 8,
>> thus why is the second one not & 0xFF ?
>> 
> 
> D'oh, I should read better. "...to form a SI32 value". Thus, this is LGTM.

Will apply.

Thanks
Steven







More information about the ffmpeg-devel mailing list