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

[Michael Niedermayer]

On Sun, Apr 21, 2019 at 11:04:00PM +0000, Andreas Rheinhardt wrote:
> 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

hmm, ok
it is still duplicated in the source though and this can mutate by being
edited by someone not knowing it should stay in sync.
so it may be better to put that string in s static const or #define
or the printing code itself could be in its own function

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190423/a3415d96/attachment.sig>