[FFmpeg-devel,1/2] avformat/yuv4mpegenc: Simplify writing global and packet headers

YUV4MPEG writes a string as header for both the file itself as well as
for every frame; these strings contain magic strings and these were up
until now included in the string to write via %s. Yet they are compile
time constants, so one can use the compile-time string concatentation
instead of inserting these strings at runtime.
Furthermore, the global header has been written via snprintf() to
a local buffer first before writing it. This can be simplified by using
avio_printf().

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
That snprintf() call was weird: The buffer used had a size of
Y4M_LINE_MAX + 1, yet snprintf has been told that the size was
Y4M_LINE_MAX, despite snprintf always adding a trailing zero (i.e. it
writes at most Y4M_LINE_MAX - 1 chars and then adds a zero).
Furthermore, snprintf only returns something negative on format errors,
not if the buffer is too small (this can be checked via the return
value). And returning AVERROR(EIO) for this error is strange, too.

 libavformat/yuv4mpegenc.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

On 9/4/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
> YUV4MPEG writes a string as header for both the file itself as well as
> for every frame; these strings contain magic strings and these were up
> until now included in the string to write via %s. Yet they are compile
> time constants, so one can use the compile-time string concatentation
> instead of inserting these strings at runtime.
> Furthermore, the global header has been written via snprintf() to
> a local buffer first before writing it. This can be simplified by using
> avio_printf().
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---


lgtm

> That snprintf() call was weird: The buffer used had a size of
> Y4M_LINE_MAX + 1, yet snprintf has been told that the size was
> Y4M_LINE_MAX, despite snprintf always adding a trailing zero (i.e. it
> writes at most Y4M_LINE_MAX - 1 chars and then adds a zero).
> Furthermore, snprintf only returns something negative on format errors,
> not if the buffer is too small (this can be checked via the return
> value). And returning AVERROR(EIO) for this error is strange, too.
>
>  libavformat/yuv4mpegenc.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/libavformat/yuv4mpegenc.c b/libavformat/yuv4mpegenc.c
> index c4781042bd..fdd020e13b 100644
> --- a/libavformat/yuv4mpegenc.c
> +++ b/libavformat/yuv4mpegenc.c
> @@ -24,18 +24,15 @@
>  #include "internal.h"
>  #include "yuv4mpeg.h"
>
> -#define Y4M_LINE_MAX 256
> -
>  static int yuv4_write_header(AVFormatContext *s)
>  {
>      AVStream *st;
>      AVIOContext *pb = s->pb;
>      int width, height;
> -    int raten, rated, aspectn, aspectd, n;
> +    int raten, rated, aspectn, aspectd, ret;
>      char inter;
>      const char *colorspace = "";
>      const char *colorrange = "";
> -    char buf[Y4M_LINE_MAX + 1];
>      int field_order;
>
>      st     = s->streams[0];
> @@ -170,19 +167,15 @@ static int yuv4_write_header(AVFormatContext *s)
>          break;
>      }
>
> -    /* construct stream header, if this is the first frame */
> -    n = snprintf(buf, Y4M_LINE_MAX, "%s W%d H%d F%d:%d I%c A%d:%d%s%s\n",
> -                 Y4M_MAGIC, width, height, raten, rated, inter,
> -                 aspectn, aspectd, colorspace, colorrange);
> -
> -    if (n < 0) {
> +    ret = avio_printf(pb, Y4M_MAGIC " W%d H%d F%d:%d I%c A%d:%d%s%s\n",
> +                      width, height, raten, rated, inter,
> +                      aspectn, aspectd, colorspace, colorrange);
> +    if (ret < 0) {
>          av_log(s, AV_LOG_ERROR,
>                 "Error. YUV4MPEG stream header write failed.\n");
> -        return AVERROR(EIO);
> +        return ret;
>      }
>
> -    avio_write(pb, buf, strlen(buf));
> -
>      return 0;
>  }
>
> @@ -200,7 +193,7 @@ static int yuv4_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>
>      /* construct frame header */
>
> -    avio_printf(s->pb, "%s\n", Y4M_FRAME_MAGIC);
> +    avio_printf(s->pb, Y4M_FRAME_MAGIC "\n");
>
>      width  = st->codecpar->width;
>      height = st->codecpar->height;
> --
> 2.20.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".