[FFmpeg-devel] [PATCH 14/15] avformat/matroskaenc: Improve log messages for blocks

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Mon Apr 22 02:04:00 EEST 2019


Michael Niedermayer:
> On Sat, Apr 20, 2019 at 01:41:09AM +0200, Andreas Rheinhardt wrote:
>> Up until now, a block's relative offset has been reported as the offset
>> in the log messages output when writing blocks; given that it is
>> impossible to know the real offset from the beginning of the file at
>> this point due to the fact that it is not yet known how many bytes will
>> be used for the containing cluster's length field both the relative
>> offset in the cluster as well as the offset of the containing cluster
>> will be reported from now on.
>>
>> Also, the log message for writing vtt blocks has been brought in line
>> with the message for normal blocks.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>>  libavformat/matroskaenc.c | 15 ++++++++-------
>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 441315e2d5..fc0e95440e 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -2124,10 +2124,10 @@ static void mkv_write_block(AVFormatContext *s, AVIOContext *pb,
>>  
>>      ts += mkv->tracks[pkt->stream_index].ts_offset;
>>  
>> -    av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size %d, "
>> -           "pts %" PRId64 ", dts %" PRId64 ", duration %" PRId64 ", keyframe %d\n",
>> -           avio_tell(pb), pkt->size, pkt->pts, pkt->dts, pkt->duration,
>> -           keyframe != 0);
>> +    av_log(s, AV_LOG_DEBUG, "Writing block at relative offset %" PRId64 " in "
>> +           "cluster at offset %" PRId64 "; size %d, pts %" PRId64 ", dts %" PRId64
>> +           ", duration %" PRId64 ", keyframe %d\n", avio_tell(pb), mkv->cluster_pos,
>> +           pkt->size, pkt->pts, pkt->dts, pkt->duration, keyframe != 0);
>>      if (par->codec_id == AV_CODEC_ID_H264 && par->extradata_size > 0 &&
>>          (AV_RB24(par->extradata) == 1 || AV_RB32(par->extradata) == 1))
>>          ff_avc_parse_nal_units_buf(pkt->data, &data, &size);
> 
>> @@ -2231,9 +2231,10 @@ static int mkv_write_vtt_blocks(AVFormatContext *s, AVIOContext *pb, AVPacket *p
>>  
>>      size = id_size + 1 + settings_size + 1 + pkt->size;
>>  
>> -    av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size %d, "
>> -           "pts %" PRId64 ", dts %" PRId64 ", duration %" PRId64 ", flags %d\n",
>> -           avio_tell(pb), size, pkt->pts, pkt->dts, pkt->duration, flags);
>> +    av_log(s, AV_LOG_DEBUG, "Writing block at relative offset %" PRId64 " in "
>> +           "cluster at offset %" PRId64 "; size %d, pts %" PRId64 ", dts %" PRId64
>> +           ", duration %" PRId64 ", keyframe %d\n", avio_tell(pb), mkv->cluster_pos,
>> +           size, pkt->pts, pkt->dts, pkt->duration, 1);
> 
> It does feel a bit odd to pass a litteral 1 as argument to %d to print a 1.
> why is that not a "1" or ommited ?
> also these 2 look a bit duplicated
> and its a unreadable spagethi, iam sure this can be made more readable with
> some line break changes

The duplication is intentional: It means that there needs to be only
one string literal in the actual binary. Given that the first
occurence of this string is used for both keyframes and non-keyframes,
this needs to be a parameter and so we have the literal 1 in the
second call.

I'll improve the formatting.

- Andreas


More information about the ffmpeg-devel mailing list